| Message ID | 20190212025632.28946-4-peterx@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | userfaultfd: write protection support | expand |
On Tue, Feb 12, 2019 at 10:56:09AM +0800, Peter Xu wrote: > The idea comes from the upstream discussion between Linus and Andrea: > > https://lkml.org/lkml/2017/10/30/560 > > A summary to the issue: there was a special path in handle_userfault() > in the past that we'll return a VM_FAULT_NOPAGE when we detected > non-fatal signals when waiting for userfault handling. We did that by > reacquiring the mmap_sem before returning. However that brings a risk > in that the vmas might have changed when we retake the mmap_sem and > even we could be holding an invalid vma structure. > > This patch removes the risk path in handle_userfault() then we will be > sure that the callers of handle_mm_fault() will know that the VMAs > might have changed. Meanwhile with previous patch we don't lose > responsiveness as well since the core mm code now can handle the > nonfatal userspace signals quickly even if we return VM_FAULT_RETRY. > > Suggested-by: Andrea Arcangeli <aarcange@redhat.com> > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> > Signed-off-by: Peter Xu <peterx@redhat.com> Reviewed-by: Jérôme Glisse <jglisse@redhat.com> > --- > fs/userfaultfd.c | 24 ------------------------ > 1 file changed, 24 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 89800fc7dc9d..b397bc3b954d 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -514,30 +514,6 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) > > __set_current_state(TASK_RUNNING); > > - if (return_to_userland) { > - if (signal_pending(current) && > - !fatal_signal_pending(current)) { > - /* > - * If we got a SIGSTOP or SIGCONT and this is > - * a normal userland page fault, just let > - * userland return so the signal will be > - * handled and gdb debugging works. The page > - * fault code immediately after we return from > - * this function is going to release the > - * mmap_sem and it's not depending on it > - * (unlike gup would if we were not to return > - * VM_FAULT_RETRY). > - * > - * If a fatal signal is pending we still take > - * the streamlined VM_FAULT_RETRY failure path > - * and there's no need to retake the mmap_sem > - * in such case. > - */ > - down_read(&mm->mmap_sem); > - ret = VM_FAULT_NOPAGE; > - } > - } > - > /* > * Here we race with the list_del; list_add in > * userfaultfd_ctx_read(), however because we don't ever run > -- > 2.17.1 >
diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 89800fc7dc9d..b397bc3b954d 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -514,30 +514,6 @@ vm_fault_t handle_userfault(struct vm_fault *vmf, unsigned long reason) __set_current_state(TASK_RUNNING); - if (return_to_userland) { - if (signal_pending(current) && - !fatal_signal_pending(current)) { - /* - * If we got a SIGSTOP or SIGCONT and this is - * a normal userland page fault, just let - * userland return so the signal will be - * handled and gdb debugging works. The page - * fault code immediately after we return from - * this function is going to release the - * mmap_sem and it's not depending on it - * (unlike gup would if we were not to return - * VM_FAULT_RETRY). - * - * If a fatal signal is pending we still take - * the streamlined VM_FAULT_RETRY failure path - * and there's no need to retake the mmap_sem - * in such case. - */ - down_read(&mm->mmap_sem); - ret = VM_FAULT_NOPAGE; - } - } - /* * Here we race with the list_del; list_add in * userfaultfd_ctx_read(), however because we don't ever run
The idea comes from the upstream discussion between Linus and Andrea: https://lkml.org/lkml/2017/10/30/560 A summary to the issue: there was a special path in handle_userfault() in the past that we'll return a VM_FAULT_NOPAGE when we detected non-fatal signals when waiting for userfault handling. We did that by reacquiring the mmap_sem before returning. However that brings a risk in that the vmas might have changed when we retake the mmap_sem and even we could be holding an invalid vma structure. This patch removes the risk path in handle_userfault() then we will be sure that the callers of handle_mm_fault() will know that the VMAs might have changed. Meanwhile with previous patch we don't lose responsiveness as well since the core mm code now can handle the nonfatal userspace signals quickly even if we return VM_FAULT_RETRY. Suggested-by: Andrea Arcangeli <aarcange@redhat.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Peter Xu <peterx@redhat.com> --- fs/userfaultfd.c | 24 ------------------------ 1 file changed, 24 deletions(-)