Re: [RFC PATCH 00/10] uaccess: Refuse and warn on user accesses when current->mm is NULL

["Arnd Bergmann" <arnd@arndb.de>]

On Wed, Oct 28, 2026, at 00:17, James Houghton wrote:
>>
>>  - does there need to be any compile-time configuration to skip
>>    the extra checks and/or the warnings?
>
> Since patch 10 moves the warning out of line, it costs nothing on the
> fast path, so making the WARN configurable wouldn't save anything. The
> question is really whether the mm check itself should be configurable,
> and that check is also what makes the access safe.

Right, makes sense.

> The text size increase seems manageable[1], but it would be great to
> hear other people's thoughts. For the runtime cost, I'll do some
> performance testing. My guess is that a load of current->mm and a
> well-predicted branch per user access will be negligible, but I'll get
> numbers.

Sounds good. I agree this should not have much of a performance
impact, and for the size it is probably as good as it gets without
a major rework of the lazy mm switch.

>>  - are there any cheaper ways to make it safe than to add a
>>    condition on each user access?
>>    E.g. the original lazy mm switch was intended to avoid
>>    expensive TLB flushes when switching to a kernel thread and
>>    back to the original task on 32-bit x86. On modern CPUs,
>>    the MMU can deal with multiple ASIDs cheaply, so turning
>>    off user access in a kernel thread may be cheaper than
>>    adding code to uaccess depending on the architecture.
>
> That's a fair point. If user memory were inaccessible in kernel threads
> and after exit_mm(), every such access would fault, and the fault
> handler could WARN about it (x86's do_user_addr_fault() already has a
> !mm branch), so it could still be loud without a check on every access.
>
> I'll look into disabling lazy mm switching on x86 and doing some
> performance testing with that (like comparing the per-uaccess check
> with the added cost of non-lazy mm switching).
>
> Other than the performance difference, I see these issues with making
> uaccess safe entirely with the mm switch mechanics:
>
>  - It only helps on architectures (and configs) that implement it,
>    while the common check works everywhere today.

Yes, this would probably be opt-in by architecture, unless we decide
to give up on lazy mm switch altogether and pay the performance
penalty for that on CPUs without ASID support. Either way it is not
be something we can do right away but that we can still consider
in the future.

>  - The fault handler would need to tell legitimate faults apart from
>    bugs, e.g. copy_from_user_nofault() from BPF or perf in a kernel
>    thread is fine and must stay quiet.

Would it be possible to move those checks into the individual
callers that want to avoid the WARN_ON(), with a comment on
each one explaining why they do that? It feels more consistent
to always warn here, and I would expect that for most of the
_nofault() users we do want the warning, and I'm still unsure
what is special about these.

> The series handles that today by
>    checking current->mm in the nofault helpers (patch 6).
>
> The two approaches are mostly complementary: if non-lazy switching
> turns out to be cheap enough, an architecture that does this will have
> better defense against uaccess bugs. But I'd still like the common
> check so that these bugs are reported on every architecture.

Agreed. My main remaining concern is about the number of functions
that need the additional check, if we want to avoid missing
any of them. If we can have the check in access_ok(), that would
cover the majority of the functions you are changing right now,
and it looks like we can deal with the corner cases individually.

Since in principle every user access should go through both
access_ok/mask_user_address and current->mm check at some point,
so combining the two as much as possible should help long-term
maintainability.

>> There are of course few enough of these left that I think
>> we can consider doing the same thing here and turn them all
>> into the checking version:
>>
>> $ git grep -wl __copy_'\(from\|to\)'_user  drivers mm virt
>> drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
>> drivers/gpu/drm/i915/i915_gem.c
>> drivers/gpu/drm/i915/i915_query.c
>> drivers/usb/gadget/udc/atmel_usba_udc.c
>> drivers/vhost/scsi.c
>> drivers/vhost/vhost.c
>> mm/kasan/kasan_test_c.c
>> virt/kvm/kvm_main.c
>
> Agreed, I think we should clean those up, but that can be done
> separately. This series already adds the check to the common
> __copy_{from,to}_user() and __copy_{from,to}_user_inatomic() wrappers
> in include/linux/uaccess.h (patches 7 and 8), so the KVM path is
> covered without converting the callers first.

Right, if dropping __copy_{from,to}_user() as global interfaces does
not help add the check in a consistent way, that can wait as well.
It looks like we are very slowly converting away from them already,
as we have about half the call sites left compared to v5.0.

There are probably few enough of these that we could address them
by either adding current->mm checks or converting them to use
the full versions instead where that is an obvious change like
that usb gadget driver.

>> As a quick fix I would probably start with just the 'if (!mm) return false'
>> bit here to get something merged while we still discuss the other
>> details.
>
> Making the same point as Sean[2] but with a little more detail, I had
> thought about making the change in access_ok(), but:
>
>  - On x86-64 and powerpc, the masked user access paths don't call
>    access_ok() at all. That includes copy_from_user(),
>    strncpy_from_user(), strnlen_user() and every scoped_user_*_access()
>    block, which is where many of the converted __get_user() users ended
>    up.

Ok. Apparently there is some of this on arm64 as well, using
__uaccess_mask_ptr() instead of mask_user_address().

>  - __copy_{from,to}_user() and __copy_from_user_inatomic() skip it too,
>    and the KVM bug followed exactly that pattern: the range was checked
>    once up front, and the access came much later from a different
>    context.

Arguably this is itself a TOCTOU issue and replacing
all __copy_{from,to}_user() with copy_{from,to}_user() would be a better
solution long-term, if we can do that.

>  - access_ok() is also used as a plain range check where having no mm
>    is fine. For example, copy_{from,to}_user_nofault() call
>    __access_ok()/access_ok(), and BPF and perf legitimately call those
>    from kernel threads, so a WARN there would fire on valid tracing.

These at least could be easily worked around by just skipping the
calls to copy_{from,to}_user_nofault() as mentioned above.

      Arnd