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

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

On Tue, Oct 27, 2026, at 18:29, James Houghton wrote:
> On Mon, Oct 26, 2026 at 5:13 PM Arnd Bergmann <arnd@arndb.de> wrote:
>>
>> E.g. on arm64 with CONFIG_ARM64_SW_TTBR0_PAN, the page table base for
>> the current process is only set in user_access_begin() and would
>> not point to any task here. Without ttbr0_pan, it should be
>> covered by cpu_set_reserved_ttbr0() turning off all user space access
>> for kernel threads.
>>
>> Should this perhaps be addressed in architecture specific code
>> instead of in a common place?
>
> You're right that whether corruption can actually occur depends on the
> architecture and config. I'll write a note about this in the v2 cover
> letter.
>
> That being said, I still think the right thing to do is to have the
> new check(s) in common code. Instances of improper uaccesses are still
> bugs, even if no corruption occurs.

I agree we have to do something to prevent the problem for most
configurations and find any newly introduced bugs.  The questions
that need to be properly decided are

 - does there need to be any compile-time configuration to skip
   the extra checks and/or the warnings?

 - 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.

>> I would think that user_access_begin() is a more natural place
>> for the added checks, as this should be called exactly once for
>> any function that accesses user space and also stays out of the
>> way of the copy_from_kernel_nofault() etc.
>
> user_access_begin() isn't used in a lot of places that I want to
> catch, including the KVM case. Roughly:
>
>   kvm_write_guest_cached() --> __copy_to_user() --> raw_copy_to_user()
>
> There's no user_access_begin() in there. :/

Good point, yes. After the changes described in
https://lwn.net/Articles/1102061/, I had assumed that all
__copy_{from,to}_user() are going from common code, but it looks
like the series so far only touches __{get,put}_user().

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

> I do think user_access_begin() should have the new check though, so
> I've added it in the v2.

I had the details confused here. Seing that user_access_begin()
and the other accessors always call access_ok() at some
point, maybe that would be enough?:

--- a/include/asm-generic/access_ok.h
+++ b/include/asm-generic/access_ok.h
@@ -33,6 +33,9 @@ static inline int __access_ok(const void __user *ptr, unsigned long size)
        unsigned long limit = TASK_SIZE_MAX;
        unsigned long addr = (unsigned long)ptr;
 
+       if (WARN_ONCE(!current->mm))
+               return false;
+
        if (IS_ENABLED(CONFIG_ALTERNATE_USER_ADDRESS_SPACE) ||
            !IS_ENABLED(CONFIG_MMU))
                return true;

(plus the few extra copies of access_ok()/__access_ok())

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.

      Arnd