Skip to content

Commit

Permalink
mm/gup: continue VM_FAULT_RETRY processing even for pre-faults
Browse files Browse the repository at this point in the history
When get_user_pages*() is called with pages = NULL, the processing of
VM_FAULT_RETRY terminates early without actually retrying to fault-in all
the pages.

If the pages in the requested range belong to a VMA that has userfaultfd
registered, handle_userfault() returns VM_FAULT_RETRY *after* user space
has populated the page, but for the gup pre-fault case there's no actual
retry and the caller will get no pages although they are present.

This issue was uncovered when running post-copy memory restore in CRIU
after d9c9ce3 ("x86/fpu: Fault-in user stack if
copy_fpstate_to_sigframe() fails").

After this change, the copying of FPU state to the sigframe switched from
copy_to_user() variants which caused a real page fault to get_user_pages()
with pages parameter set to NULL.

In post-copy mode of CRIU, the destination memory is managed with
userfaultfd and lack of the retry for pre-fault case in get_user_pages()
causes a crash of the restored process.

Making the pre-fault behavior of get_user_pages() the same as the "normal"
one fixes the issue.

Link: http://lkml.kernel.org/r/[email protected]
Fixes: d9c9ce3 ("x86/fpu: Fault-in user stack if copy_fpstate_to_sigframe() fails")
Signed-off-by: Mike Rapoport <[email protected]>
Tested-by: Andrei Vagin <[email protected]> [https://travis-ci.org/avagin/linux/builds/533184940]
Tested-by: Hugh Dickins <[email protected]>
Cc: Andrea Arcangeli <[email protected]>
Cc: Sebastian Andrzej Siewior <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Pavel Machek <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
rppt authored and torvalds committed Jun 1, 2019
1 parent b9fba67 commit df17277
Showing 1 changed file with 8 additions and 7 deletions.
15 changes: 8 additions & 7 deletions mm/gup.c
Original file line number Diff line number Diff line change
Expand Up @@ -1042,10 +1042,6 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
BUG_ON(ret >= nr_pages);
}

if (!pages)
/* If it's a prefault don't insist harder */
return ret;

if (ret > 0) {
nr_pages -= ret;
pages_done += ret;
Expand All @@ -1061,8 +1057,12 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
pages_done = ret;
break;
}
/* VM_FAULT_RETRY triggered, so seek to the faulting offset */
pages += ret;
/*
* VM_FAULT_RETRY triggered, so seek to the faulting offset.
* For the prefault case (!pages) we only update counts.
*/
if (likely(pages))
pages += ret;
start += ret << PAGE_SHIFT;

/*
Expand All @@ -1085,7 +1085,8 @@ static __always_inline long __get_user_pages_locked(struct task_struct *tsk,
pages_done++;
if (!nr_pages)
break;
pages++;
if (likely(pages))
pages++;
start += PAGE_SIZE;
}
if (lock_dropped && *locked) {
Expand Down

0 comments on commit df17277

Please sign in to comment.