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

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 2504 bytes --]

> 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@google.com>

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?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40410166379