__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 <[email protected]> --- include/linux/uaccess.h | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h index 8ab24e706a22..eae204f7261b 100644 --- a/include/linux/uaccess.h +++ b/include/linux/uaccess.h @@ -51,20 +51,33 @@ #endif /* - * Return true if a user copy must not be attempted, either because a failure - * is being injected, or because the current task has no mm. + * Return true, and warn, if the current task has no mm, in which case user + * memory must not be accessed. * * Accessing user memory without an mm (e.g. from a kernel thread that is not * using kthread_use_mm(), or from a task that has already gone through * exit_mm()) is always a bug: the user portion of the address space belongs to * whatever mm happens to be lazily loaded, if any, so the access could * silently hit another task's memory. + * + * Note that this only catches a missing mm. It cannot catch a valid mm that + * isn't the one the caller expects, e.g. when a file's release or ioctl + * handlers run in a different process than the one that created it. Code + * that keeps user addresses belonging to a specific mm must check + * current->mm against that mm itself (see e.g. KVM's kvm->mm checks). + */ +static __always_inline bool uaccess_without_mm(void) +{ + return WARN_ON_ONCE(!current->mm); +} + +/* + * Return true if a user copy must not be attempted, either because the + * current task has no mm, or because a failure is being injected. */ static __always_inline bool uaccess_should_fail(void) { - if (WARN_ON_ONCE(!current->mm)) - return true; - return should_fail_usercopy(); + return uaccess_without_mm() || should_fail_usercopy(); } /* @@ -117,6 +130,8 @@ __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n) { unsigned long res; + if (uaccess_without_mm()) + return n; instrument_copy_from_user_before(to, from, n); check_object_size(to, n, false); res = raw_copy_from_user(to, from, n); -- 2.56.0.307.g5fe7bd680c-goog
