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

[James Houghton <jthoughton@google.com>]

On Mon, Oct 26, 2026 at 3:42 PM <bot+bpf-ci@kernel.org> wrote:
>
> > uaccess: Refuse and warn on user copies when current->mm is NULL
>
> TL;DR:
> - Possible spurious -EFAULT on valid kernel addresses on xtensa/nios2 from
>   mm-less tasks (kworkers), e.g. d_path
> - nofault helpers may hit the new WARN_ON_ONCE (BPF/tracing); fix is later in
>   series, breaking bisection
>
> > diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> > index eddbbb65ccc4..e54a6d804bf9 100644
> > --- a/include/linux/uaccess.h
> > +++ b/include/linux/uaccess.h
> > @@ -50,6 +50,23 @@
>
> [ ... ]
>
> > +static __always_inline bool uaccess_should_fail(void)
> > +{
> > +     if (WARN_ON_ONCE(!current->mm))
> > +             return true;
> > +     return should_fail_usercopy();
> > +}
>
> [ ... ]
>
> > @@ -114,7 +131,7 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
> >
> >       might_fault();
> >       instrument_copy_from_user_before(to, from, n);
> > -     if (should_fail_usercopy())
> > +     if (uaccess_should_fail())
> >               return n;
> >       check_object_size(to, n, false);
> >       res = raw_copy_from_user(to, from, n);
>
> Can this refuse legitimate kernel address copies on xtensa and nios2?
>
> On both architectures __copy_from_user() and __copy_to_user() also sit under
> the generic kernel nofault accessors.  Neither architecture defines
> __get_kernel_nofault or arch_get_kernel_nofault, so both use the fallback in
> include/linux/uaccess.h, which goes through __get_user()/__put_user() on the
> kernel address:
>
>     #define __get_kernel_nofault(dst, src, type, label)    \
>     ...
>             if (__get_user(data, p))                       \
>                     goto label;                            \
>
> On xtensa, 8-byte __get_user()/__put_user() are implemented with
> __copy_from_user()/__copy_to_user() (arch/xtensa/include/asm/uaccess.h):
>
>     case 8: {
>             u64 __x;
>             if (unlikely(__copy_from_user(&__x, ptr, 8))) {
>     ...
>             retval = __copy_to_user(ptr, &__v64, 8) ? -EFAULT : 0;
>
> On nios2, __put_user_common() sends any size other than 1, 2 or 4 to
> __copy_to_user().
>
> Neither architecture selects HAVE_EFFICIENT_UNALIGNED_ACCESS, so
> copy_from_kernel_nofault() and copy_to_kernel_nofault() (mm/maccess.c) use
> the u64 loop whenever dst and src are both 8-byte aligned and at least 8
> bytes remain:
>
>     if (!(align & 7))
>             copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
>
> With this patch, every such access from a task with current->mm == NULL
> (kworkers, khungtaskd, kernel_init before exec, the idle task in IRQ, kgdb
> entered from idle) hits WARN_ON_ONCE(!current->mm), and __copy_from_user()
> returns n.  copy_from_kernel_nofault() then returns -EFAULT on valid kernel
> memory.
>
> Concrete generic callers include:
>
>   d_path() -> prepend() -> prepend_copy() (fs/d_path.c) fills the component
>   with 'x' and stops the walk.  This is reachable from LSM/IMA path hooks
>   run by kworkers, for example async firmware loading.
>
>   print_worker_info() (kernel/workqueue.c), called from
>   dump_stack_print_info() and sched_show_task() for kworkers, which copies
>   wq->name and worker->desc.
>
>   copy_to_kernel_nofault() from kgdb_hex2mem()/kgdb_ebin2mem() on nios2,
>   which selects HAVE_ARCH_KGDB.
>
> Before this patch these accesses worked on kernel addresses, since
> raw_copy_{from,to}_user() on these architectures is a plain fixup-protected
> copy and access_ok() is not involved.  Only opt-in fault injection could
> make them fail.
>
> The commit message rationale ("Accessing user memory from a task that has no
> mm is always a bug") does not apply to kernel addresses.  The commit message
> also says get_user()/put_user() are not covered, which does not hold for
> 8-byte accesses on these two architectures.
>
> No later commit in this series appears to touch these paths.

This analysis is correct. Implementing copy-from-kernel functions with calls to
uaccess functions is a little hard to make sense of. :/

I've included a patch at the front of my v2 for xtensa and nios2. I don't think
any other architecture has this problem. I've placed the patch at the bottom of
this reply. I've also adjusted the wording for patch 6.

>
> [ ... ]
>
> > diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
> > index 5bb752ff7c61..565b3125e752 100644
> > --- a/lib/strncpy_from_user.c
> > +++ b/lib/strncpy_from_user.c
> > @@ -115,7 +115,7 @@ long strncpy_from_user(char *dst, const char __user *src, long count)
> >       unsigned long max_addr, src_addr;
> >
> >       might_fault();
> > -     if (should_fail_usercopy())
> > +     if (uaccess_should_fail())
> >               return -EFAULT;
> >       if (unlikely(count <= 0))
> >               return 0;
>
> Can strncpy_from_user_nofault() now trip this WARN_ON_ONCE() from contexts
> where current->mm is NULL by design?
>
> In this tree strncpy_from_user_nofault() (mm/maccess.c) has no current->mm
> check and no nmi_uaccess_okay() check.  It goes straight to
> strncpy_from_user():
>
>     pagefault_disable();
>     ret = strncpy_from_user(dst, unsafe_addr, count);
>     pagefault_enable();
>
> and strncpy_from_user() now calls uaccess_should_fail(), which does:
>
>     if (WARN_ON_ONCE(!current->mm))
>             return true;
>
> The callers of strncpy_from_user_nofault() are tracing helpers that run in
> whatever context the probe fires in, including kernel threads and IRQs taken
> while the idle task runs:
>
>   bpf_probe_read_user_str() via bpf_probe_read_user_str_common()
>   (kernel/trace/bpf_trace.c), and bpf_probe_read_str() for addresses below
>   TASK_SIZE.
>
>   bpf_trace_printk()/bpf_snprintf() through bpf_trace_copy_string()
>   (kernel/bpf/helpers.c).  With CONFIG_ARCH_HAS_NON_OVERLAPPING_ADDRESS_SPACE
>   (x86, arm64, ...), any '%s' argument below TASK_SIZE takes this path,
>   including a NULL pointer.  So bpf_printk("%s", ptr) with ptr == NULL in a
>   kprobe that fires in a kworker now triggers the warning.
>
>   kprobe/fprobe/eprobe ustring fetch args (fetch_store_string_user() in
>   kernel/trace/trace_probe_kernel.h).
>
>   The .ustring event filters (test_ustring() in
>   kernel/trace/trace_events_filter.c).
>
>   Synthetic event string fields (trace_string() in
>   kernel/trace/trace_events_synth.c).
>
>   The bpf_copy_from_user_str_dynptr family (copy_user_str_nofault()).
>
> Before this patch these calls either failed quietly with -EFAULT or read the
> lazily loaded mm.  Now a tracing user (CAP_BPF plus CAP_PERFMON, or tracefs
> access) can trigger a warning splat, and a panic with panic_on_warn, just by
> tracing a function that kernel threads run.
>
> The same applies to copy_to_user_iter_nofault() in lib/iov_iter.c, which this
> patch also switches to uaccess_should_fail().  The commit message lists
> copy_to_user_nofault() as intentionally covered, but it does not account for
> the nofault helpers being used from mm-less contexts by design.
>
> A fix appears later in the series, e8bf23b5b30d ("uaccess: Fail user nofault
> helpers quietly when current->mm is NULL").  It adds an early !current->mm
> return to copy_{from,to}_user_nofault(), strncpy_from_user_nofault() and
> strnlen_user_nofault(), and switches copy_to_user_iter_nofault() back to
> should_fail_usercopy().  Its message describes this exact problem
> (bpf_probe_read_user_str() from a BPF program attached to a function called
> by a kernel thread would now trip the WARN_ON_ONCE() in
> strncpy_from_user()).
>
> This commit on its own still introduces the regression, which breaks
> bisection.  Could that fix be folded into this patch, or ordered before it?

Ah, yes. I'll flip the order of patch 6 and 7 in the v2. Thanks.

Okay, the xtensa/nios2 patch is here:

-------

    xtensa, nios2: Don't use __copy_{from,to}_user() in __{get,put}_user()
    
    Architectures that don't define __get_kernel_nofault() or
    arch_get_kernel_nofault() use the generic fallback in
    include/linux/uaccess.h, which accesses *kernel* addresses with
    __get_user() and __put_user(). That relies on __get_user() and
    __put_user() being raw accessors, i.e. plain fixup-protected loads and
    stores with no extra checks that only make sense for user memory.
    
    xtensa and nios2 use this fallback, but don't quite meet that: on
    xtensa, 8-byte __get_user() and __put_user() are implemented with
    __copy_from_user() and __copy_to_user(), and on nios2, __put_user() of
    any size other than 1, 2 or 4 bytes (i.e. 8-byte puts) uses
    __copy_to_user(). __copy_{from,to}_user() do more than a plain copy: they
    inject failures with CONFIG_FAULT_INJECTION_USERCOPY, check the kernel
    object size with CONFIG_HARDENED_USERCOPY, and, with a later patch in
    this series, refuse and warn on user accesses when current->mm is NULL.
    
    As neither architecture selects HAVE_EFFICIENT_UNALIGNED_ACCESS,
    copy_from_kernel_nofault() and copy_to_kernel_nofault() use 8-byte
    accesses when both pointers are 8-byte aligned, so kernel nofault
    accesses (used e.g. by d_path() and print_worker_info()) end up in
    __copy_{from,to}_user() on kernel addresses. With the NULL mm check,
    copy_from_kernel_nofault() would then fail on valid kernel memory when
    called from kernel threads.
    
    Use raw_copy_{from,to}_user() instead, as asm-generic's
    __get_user_fn()/__put_user_fn() do, and as nios2 already does for 8-byte
    gets (__get_user_8()). This only drops the extra checks and KASAN/KCSAN
    instrumentation for 8-byte accesses, making them consistent with the 1,
    2 and 4-byte cases, which are plain inline asm.
    
    The fallback itself is fine and is still needed: it's used by alpha,
    arc, csky, hexagon, microblaze, nios2, openrisc, sh, sparc and xtensa.
    Of those, only xtensa and nios2 have __get_user()/__put_user() that
    aren't raw. The others use either asm-generic's, which call
    raw_copy_{from,to}_user(), or inline asm for all sizes. The remaining
    architectures (arm, arm64, loongarch, m68k, mips, parisc, powerpc,
    riscv, s390, um and x86) define their own __get_kernel_nofault() or
    arch_get_kernel_nofault(), which don't use __get_user()/__put_user() or
    __copy_{from,to}_user().

    Signed-off-by: James Houghton <jthoughton@google.com>
    
diff --git a/arch/nios2/include/asm/uaccess.h b/arch/nios2/include/asm/uaccess.h
index 5e6e05cc6efc..f20f11bd21dc 100644
--- a/arch/nios2/include/asm/uaccess.h
+++ b/arch/nios2/include/asm/uaccess.h
@@ -161,7 +161,7 @@ do {									\
 	default:							\
 		/* XXX: This looks wrong... */				\
 		__pu_err = 0;						\
-		if (__copy_to_user(__pu_ptr, &(__pu_val),		\
+		if (raw_copy_to_user(__pu_ptr, &(__pu_val),		\
 			sizeof(*__pu_ptr)))				\
 			__pu_err = -EFAULT;				\
 		break;							\
diff --git a/arch/xtensa/include/asm/uaccess.h b/arch/xtensa/include/asm/uaccess.h
index 6538a29a2bbd..30c8805d1c83 100644
--- a/arch/xtensa/include/asm/uaccess.h
+++ b/arch/xtensa/include/asm/uaccess.h
@@ -76,7 +76,7 @@ do {									\
 	case 4: __put_user_asm(x, ptr, retval, 4, "s32i", __cb); break;	\
 	case 8: {							\
 		     __typeof__(*ptr) __v64 = x;			\
-		     retval = __copy_to_user(ptr, &__v64, 8) ? -EFAULT : 0;	\
+		     retval = raw_copy_to_user(ptr, &__v64, 8) ? -EFAULT : 0;	\
 		     break;						\
 	        }							\
 	default: __put_user_bad();					\
@@ -176,7 +176,7 @@ do {									\
 	case 4: __get_user_asm(x, ptr, retval, 4, "l32i", __cb);  break;\
 	case 8: {							\
 		u64 __x;						\
-		if (unlikely(__copy_from_user(&__x, ptr, 8))) {		\
+		if (unlikely(raw_copy_from_user(&__x, ptr, 8))) {	\
 			retval = -EFAULT;				\
 			(x) = (__typeof__(*(ptr)))0;			\
 		} else {						\