mm/gup: continue VM_FAULT_RETRY processing even for pre-faults
authorMike Rapoport <rppt@linux.ibm.com>
Sat, 1 Jun 2019 05:30:33 +0000 (22:30 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Sat, 1 Jun 2019 22:51:31 +0000 (15:51 -0700)
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 d9c9ce34ed5c ("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/1557844195-18882-1-git-send-email-rppt@linux.ibm.com
Fixes: d9c9ce34ed5c ("x86/fpu: Fault-in user stack if copy_fpstate_to_sigframe() fails")
Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
Tested-by: Andrei Vagin <avagin@gmail.com> [https://travis-ci.org/avagin/linux/builds/533184940]
Tested-by: Hugh Dickins <hughd@google.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Pavel Machek <pavel@ucw.cz>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
mm/gup.c

index f173fcbaf1b2c9ab40dd65a6b2f3517aa7790ab5..ddde097cf9e4106bc02ea55538926f44ed8e587c 100644 (file)
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -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;
@@ -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;
 
                /*
@@ -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) {