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

[James Houghton]

__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>
---
 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