Re: [RFC PATCH 08/10] uaccess: Refuse and warn on __copy_from_user_inatomic() without an mm

[James Houghton]

On Mon, Oct 26, 2026 at 3:30 PM <bot+bpf-ci@xxxxxxxxxx> wrote:
>
> > uaccess: Refuse and warn on __copy_from_user_inatomic() without an mm
>
> TL;DR: Commit message only: it names kvm_read_guest_atomic(), which no longer
> exists in this tree; the review suggests kvm_vcpu_read_guest_atomic() instead.
>
> >     uaccess: Refuse and warn on __copy_from_user_inatomic() without an mm
> >
> >     __copy_from_user_inatomic() never called should_fail_usercopy(), so the
> >     NULL current->mm check added to uaccess_should_fail() does not cover it.
> >     It is used e.g. by KVM's kvm_read_guest_atomic(), by several
> >     architectures' perf callchain and stack trace code, and by the generic
> >     copy_from_user_inatomic_nontemporal().
> >
> >     Split the NULL mm check out of uaccess_should_fail() into
> >     uaccess_without_mm(), and use it in __copy_from_user_inatomic() to warn
> >     and refuse the copy.
> >
> >     Don't use uaccess_should_fail() there, as that would also start injecting
> >     failures into __copy_from_user_inatomic(), which some callers don't
> >     expect to fail in practice, e.g. __wp_page_copy_user() warns if the copy
> >     fails twice.
> >
> >     copy_from_user_nofault() already fails quietly when current->mm is NULL,
> >     and the generic user stack unwinding entry points (perf_callchain(),
> >     get_perf_callchain() and stack_trace_save_user()) only unwind the user
> >     stack of a user task with an mm, so neither is affected.
> >
> >     Signed-off-by: James Houghton <jthoughton@xxxxxxxxxx>
>
> The changelog names kvm_read_guest_atomic() as a user of
> __copy_from_user_inatomic(), but that function no longer exists in this tree.
> It was removed by commit ef82eddc0e31 ("KVM: Remove kvm_read_guest_atomic()").
>
> The KVM user is now kvm_vcpu_read_guest_atomic() in virt/kvm/kvm_main.c,
> which calls the static __kvm_read_guest_atomic():
>
>     static int __kvm_read_guest_atomic(struct kvm_memory_slot *slot, gfn_t gfn,
>                                        void *data, int offset, unsigned long len)
>     {
>         ...
>         pagefault_disable();
>         r = __copy_from_user_inatomic(data, (void __user *)addr + offset, len);
>         pagefault_enable();
>
> A grep for kvm_read_guest_atomic only finds __kvm_read_guest_atomic() and
> kvm_vcpu_read_guest_atomic().
>
> Could the changelog say kvm_vcpu_read_guest_atomic() instead?

Yes, that is correct. I've applied this fix in v2.