Thread (36 messages) flat view 36 messages, 12 authors, 2016-03-08
STALE3803d

Revision v2 of 4 in this series.

Revisions (4)
  1. v1 [diff vs current]
  2. v2 [diff vs current]
  3. v2 current
  4. v2 [diff vs current]

[PATCH v2 0/5] arm64: kernel: Add support for User Access Override

From: james.morse@arm.com (James Morse)
Date: 2016-03-07 16:43:19
Subsystem: arm64 port (aarch64 architecture), the rest · Maintainers: Catalin Marinas, Will Deacon, Linus Torvalds

Hi Catalin,

I've just spotted UAO causes the test_user_copy module (CONFIG_TEST_USER_COPY)
to fail. Who to blame is up for discussion. The test is passing a user pointer
as the 'to' field of copy_from_user(), which it expects to fail gracefully:

lib/test_user_copy.c:75
/* Invalid usage: none of these should succeed. */
[ ... ]
	ret |= test(!copy_from_user(bad_usermem, (char __user *)kmem,
			    PAGE_SIZE),
	    "illegal reversed copy_from_user passed");
access_ok() catches the "(char __user *)kmem", causing copy_from_user() to pass
bad_usermem to memset():

arch/arm64/include/asm/uaccess.h:279
if (access_ok(VERIFY_READ, from, n))
	n = __copy_from_user(to, from, n);
else /* security hole - plug it */
	memset(to, 0, n);
This (correctly) trips UAO's "Accessing user space memory outside uaccess.h
routines" message, which is a little confusing to debug, and stops the rest of
the module's tests from being run.

As far as I can see, this would only affect arm64. I can't find an equivalent
memset() for x86_64.

The below ugly hack [0], handles this more gracefully. I can send this as a fix
sooner/later if you think its the right thing to do.



Thanks,

James

[0]
-----------------%<-----------------
diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h
index 0685d74572af..049a82e8dd9e 100644
--- a/arch/arm64/include/asm/uaccess.h
+++ b/arch/arm64/include/asm/uaccess.h
@@ -278,8 +278,8 @@ static inline unsigned long __must_check copy_from_user(void
*to, const void __u
 {
        if (access_ok(VERIFY_READ, from, n))
                n = __copy_from_user(to, from, n);
-       else /* security hole - plug it */
-               memset(to, 0, n);
+       else if ((unsigned long)to > USER_DS) /* swapped from/to args? */
+               memset(to, 0, n); /* security hole - plug it */
        return n;
 }

-----------------%<-----------------
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help