Re: [RFC PATCH] userfaultfd: Address race after fault.

[Brian Geffon <bgeffon@google.com>]

Hi Andrea,
That all makes sense, thanks so much for that detailed explanation.

Brian

On Mon, Feb 17, 2020 at 8:27 PM Andrea Arcangeli <aarcange@redhat.com> wrote:
>
> On Mon, Feb 17, 2020 at 07:50:19PM -0600, Brian Geffon wrote:
> > But in the meantime, if the plan of record will be to always allow
> > retrying then shouldn't the block I mailed a patch on be removed
> > regardless because do_user_addr_fault always starts with
> > FAULT_FLAG_ALLOW_RETRY and we shouldn't ever land there without it in
> > the future and allows userfaultfd to retry?
>
> It might hide the limitation but only if the page fault originated in
> userland (Android's case), but that's not something userfault users
> should depend on. Userfaults (unlike sigsegv trapping) are meant to be
> reliable and transparent to all user and kernel accesses alike.
>
> It is also is unclear how long Android will be forced to keep doing
> bounce buffers copies in RAM before considering passing any memory to
> kernel syscalls.
>
> For all other users where the kernel access may be the one triggering
> the fault the patch will remove a debug aid and the kernel fault would
> then fail by hitting on the below:
>
>                 /* Not returning to user mode? Handle exceptions or die: */
>                 no_context(regs, hw_error_code, address, SIGBUS, BUS_ADRERR);
>
> There may be more side effects in other archs I didn't evaluate
> because there's no other place where the common code can return
> VM_FAULT_RETRY despite the arch code explicitly told the common code
> it can't do that (by not setting FAULT_FLAG_ALLOW_RETRY) so it doesn't
> look very safe and it doesn't seem a generic enough solution to the
> problem.
>
> That dump_stack() helped a lot to identify those kernel outliers that
> erroneously use get_user_pages instead of the gup_locked/unlocked
> variant that are uffd-capable.
>
> Thanks,
> Andrea
>