aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorKees Cook <keescook@chromium.org>2017-02-14 12:38:07 -0800
committerKees Cook <keescook@chromium.org>2017-02-21 11:59:38 -0800
commit4c5d7bc63775b40631b75f6c59a3a3005455262d (patch)
treef87807d3452c42476285a1ffcbf72540016bf60e
parentf5f893c57e37ca730808cb2eee3820abd05e7507 (diff)
usercopy: Add tests for all get_user() sizes
The existing test was only exercising native unsigned long size get_user(). For completeness, we should check all sizes. But we must skip some 32-bit architectures that don't implement a 64-bit get_user(). These new tests actually uncovered a bug in ARM's 64-bit get_user() zeroing. Signed-off-by: Kees Cook <keescook@chromium.org>
-rw-r--r--lib/test_user_copy.c89
1 files changed, 76 insertions, 13 deletions
diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
index 73ff7a6..6f335a3 100644
--- a/lib/test_user_copy.c
+++ b/lib/test_user_copy.c
@@ -25,6 +25,23 @@
#include <linux/uaccess.h>
#include <linux/vmalloc.h>
+/*
+ * Several 32-bit architectures support 64-bit {get,put}_user() calls.
+ * As there doesn't appear to be anything that can safely determine
+ * their capability at compile-time, we just have to opt-out certain archs.
+ */
+#if BITS_PER_LONG == 64 || (!defined(CONFIG_AVR32) && \
+ !defined(CONFIG_BLACKFIN) && \
+ !defined(CONFIG_M32R) && \
+ !defined(CONFIG_M68K) && \
+ !defined(CONFIG_MICROBLAZE) && \
+ !defined(CONFIG_MN10300) && \
+ !defined(CONFIG_NIOS2) && \
+ !defined(CONFIG_PPC32) && \
+ !defined(CONFIG_SUPERH))
+# define TEST_U64
+#endif
+
#define test(condition, msg) \
({ \
int cond = (condition); \
@@ -40,7 +57,12 @@ static int __init test_user_copy_init(void)
char __user *usermem;
char *bad_usermem;
unsigned long user_addr;
- unsigned long value = 0x5A;
+ u8 val_u8;
+ u16 val_u16;
+ u32 val_u32;
+#ifdef TEST_U64
+ u64 val_u64;
+#endif
kmem = kmalloc(PAGE_SIZE * 2, GFP_KERNEL);
if (!kmem)
@@ -61,14 +83,39 @@ static int __init test_user_copy_init(void)
/*
* Legitimate usage: none of these copies should fail.
*/
- ret |= test(copy_from_user(kmem, usermem, PAGE_SIZE),
- "legitimate copy_from_user failed");
+ memset(kmem, 0x3a, PAGE_SIZE * 2);
ret |= test(copy_to_user(usermem, kmem, PAGE_SIZE),
"legitimate copy_to_user failed");
- ret |= test(get_user(value, (unsigned long __user *)usermem),
- "legitimate get_user failed");
- ret |= test(put_user(value, (unsigned long __user *)usermem),
- "legitimate put_user failed");
+ memset(kmem, 0x0, PAGE_SIZE);
+ ret |= test(copy_from_user(kmem, usermem, PAGE_SIZE),
+ "legitimate copy_from_user failed");
+ ret |= test(memcmp(kmem, kmem + PAGE_SIZE, PAGE_SIZE),
+ "legitimate usercopy failed to copy data");
+
+#define test_legit(size, check) \
+ do { \
+ val_##size = check; \
+ ret |= test(put_user(val_##size, (size __user *)usermem), \
+ "legitimate put_user (" #size ") failed"); \
+ val_##size = 0; \
+ ret |= test(get_user(val_##size, (size __user *)usermem), \
+ "legitimate get_user (" #size ") failed"); \
+ ret |= test(val_##size != check, \
+ "legitimate get_user (" #size ") failed to do copy"); \
+ if (val_##size != check) { \
+ pr_info("0x%llx != 0x%llx\n", \
+ (unsigned long long)val_##size, \
+ (unsigned long long)check); \
+ } \
+ } while (0)
+
+ test_legit(u8, 0x5a);
+ test_legit(u16, 0x5a5b);
+ test_legit(u32, 0x5a5b5c5d);
+#ifdef TEST_U64
+ test_legit(u64, 0x5a5b5c5d6a6b6c6d);
+#endif
+#undef test_legit
/*
* Invalid usage: none of these copies should succeed.
@@ -105,12 +152,28 @@ static int __init test_user_copy_init(void)
PAGE_SIZE),
"illegal reversed copy_to_user passed");
- value = 0x5a;
- ret |= test(!get_user(value, (unsigned long __user *)kmem),
- "illegal get_user passed");
- ret |= test(value != 0, "zeroing failure for illegal get_user");
- ret |= test(!put_user(value, (unsigned long __user *)kmem),
- "illegal put_user passed");
+#define test_illegal(size, check) \
+ do { \
+ val_##size = (check); \
+ ret |= test(!get_user(val_##size, (size __user *)kmem), \
+ "illegal get_user (" #size ") passed"); \
+ ret |= test(val_##size != (size)0, \
+ "zeroing failure for illegal get_user (" #size ")"); \
+ if (val_##size != (size)0) { \
+ pr_info("0x%llx != 0\n", \
+ (unsigned long long)val_##size); \
+ } \
+ ret |= test(!put_user(val_##size, (size __user *)kmem), \
+ "illegal put_user (" #size ") passed"); \
+ } while (0)
+
+ test_illegal(u8, 0x5a);
+ test_illegal(u16, 0x5a5b);
+ test_illegal(u32, 0x5a5b5c5d);
+#ifdef TEST_U64
+ test_illegal(u64, 0x5a5b5c5d6a6b6c6d);
+#endif
+#undef test_illegal
vm_munmap(user_addr, PAGE_SIZE * 2);
kfree(kmem);