Re: [PATCH RFC 02/24] mm: userfault: return VM_FAULT_RETRY on signals
From: Peter Xu
Date: Tue Jan 22 2019 - 01:11:01 EST
On Mon, Jan 21, 2019 at 10:40:18AM -0500, Jerome Glisse wrote:
> On Mon, Jan 21, 2019 at 03:57:00PM +0800, Peter Xu wrote:
> > 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. The problem was reported by syzbot.
> This is confusing this should be a patch on its own ie changes to
> fs/userfaultfd.c where you remove that path.
Sure I will.
> > This patch removes the special path and we'll return a VM_FAULT_RETRY
> > with the common path even if we have got such signals. Then for all the
> > architectures that is passing in VM_FAULT_ALLOW_RETRY into
> > handle_mm_fault(), we check not only for SIGKILL but for all the rest of
> > userspace pending signals right after we returned from
> > handle_mm_fault().
> > The idea comes from the upstream discussion between Linus and Andrea:
> > https://lkml.org/lkml/2017/10/30/560
> > (This patch contains a potential fix for a double-free of mmap_sem on
> > ARC architecture; please see https://lkml.org/lkml/2018/11/1/723 for
> > more information)
> This patch should only be about changing the return to userspace rule.
> Before this patch the arch fault handler returned to userspace only
> for fatal signal, after this patch it returns to userspace for any
Ok. I'll make the first patch to do the signal changes, then the
second patch to remove the userfault path explicitly.
> It would be a lot better to have a fix for arc as a separate patch so
> that we can focus on reviewing only one thing.
I just noticed that it was fixed just a few days ago in commit
4d447455e73b. Then I'll just simply rebase to Linus master and use
the upstream fix, then I can drop this paragraph.
Thanks for the review!