On Mon, Oct 26, 2026, at 22:52, James Houghton wrote:
>
> This series comes out of a set of bugs that were found in KVM where the
> kernel can sometimes write to a random process's user memory[1]. The KVM
> fixes at [1] are still required, even with this uaccess series, as noted
> by Sean here[2] and below in this cover letter. The beginning of this
> series includes fixes for similar bugs found through static analysis
> with Claude Opus 5.5. I don't think any of these bugs are more severe
> than the KVM bugs[1].
>
> We hit one of the KVM bugs in production, and it was challenging to
> debug. This series makes such bugs, where the kernel is doing uaccess
> without an mm, much more obvious.

Nice work!

> Such an access does not reliably fail. The user portion of the address
> space belongs to whatever mm happens to be lazily loaded, if any. If
> the access faults, the fault handler refuses to handle it and the copy
> fails, but if the page is already mapped, the access silently reads or
> writes another task's memory.

This sounds like the problem depends a lot on the architecture,
and to what degree they implement lazy mm switching. If an architecture
makes all user pointers invalid for a kernel thread, this does not
actually happen, right?

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?

> == The New WARN ==
>
> The rest of the series makes the generic uaccess helpers WARN_ON_ONCE()
> and refuse the access when current->mm is NULL:
>
>  - Patch 6 adds uaccess_should_fail(), which combines the existing
>    should_fail_usercopy() fault injection check with the NULL mm check,
>    and uses it in place of should_fail_usercopy(). This covers
>    copy_{to,from}_user() on all architectures (including the masked
>    user access paths, which never call access_ok()), the
>    __copy_{to,from}_user() variants used by KVM's guest memory
>    accessors, the user-backed iov_iter copy routines and
>    strncpy_from_user().
...
> The checks are deliberately limited to generic code. Accesses
> implemented entirely by architectures (get_user(), put_user(),
> clear_user(), and direct users of user_access_begin() or
> masked_user_access_begin()) are not covered. Please let me know if the
> new check should be placed elsewhere.

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.

       Arnd

Reply via email to