When the destination buffer size is known at build time but the runtime
size to copy into it is not known, the copy_from_user() will WARN when
it is too large and the copy_from_user() will fail. However, it was not
zeroing the destination buffer (for which it knows the correct size). This
fixes that corner case and adds a test for it in test_user_copy.c.
Cc: Al Viro <v...@zeniv.linux.org.uk>
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
include/linux/uaccess.h | 5 +++--
lib/test_user_copy.c | 12 ++++++++++++
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 201418d5e15c..e350d81eb763 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -152,9 +152,10 @@ copy_from_user(void *to, const void __user *from, unsigned
long n)
if (likely(sz < 0 || sz >= n)) {
check_object_size(to, n, false);
n = _copy_from_user(to, from, n);
- } else if (!__builtin_constant_p(n))
+ } else if (!__builtin_constant_p(n)) {
copy_user_overflow(sz, n);
- else
+ memset(to, 0, sz);
+ } else
__bad_copy_user();
return n;
diff --git a/lib/test_user_copy.c b/lib/test_user_copy.c
index 4621db801b23..c7a1da157c53 100644
--- a/lib/test_user_copy.c
+++ b/lib/test_user_copy.c
@@ -57,6 +57,8 @@ static int __init test_user_copy_init(void)
char __user *usermem;
char *bad_usermem;
unsigned long user_addr;
+ volatile int unconst = 0;
+ char charbuf[8];
u8 val_u8;
u16 val_u16;
u32 val_u32;
@@ -124,6 +126,7 @@ static int __init test_user_copy_init(void)
/* Prepare kernel memory with check values. */
memset(kmem, 0x5a, PAGE_SIZE);
memset(kmem + PAGE_SIZE, 0, PAGE_SIZE);
+ memset(charbuf, 0x6a, sizeof(charbuf));
/* Reject kernel-to-kernel copies through copy_from_user(). */
ret |= test(!copy_from_user(kmem, (char __user *)(kmem + PAGE_SIZE),
@@ -134,6 +137,15 @@ static int __init test_user_copy_init(void)
ret |= test(memcmp(kmem + PAGE_SIZE, kmem, PAGE_SIZE),
"zeroing failure for illegal all-kernel copy_from_user");
+ /* Reject copies into too-small buffers. */
+ ret |= test(!copy_from_user(charbuf, usermem,
+ sizeof(charbuf) + 1 + unconst),
+ "illegal too-large copy_from_user passed");
+
+ /* Destination buffer should have been entirely zeroed. */
+ ret |= test(memcmp(kmem + PAGE_SIZE, charbuf, sizeof(charbuf)),
+ "zeroing failure for illegal too-large copy_from_user");
+
#if 0
/*
* When running with SMAP/PAN/etc, this will Oops the kernel
--
2.7.4
--
Kees Cook
Pixel Security
