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

[Andrea Arcangeli <aarcange@redhat.com>]

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