linux-kernel - [PATCH] uaccess: Zero destination buffer on overflow attempt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20170705200113.GA146915@beast>
Date:   Wed, 5 Jul 2017 13:01:13 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Al Viro <viro@...iv.linux.org.uk>, Andi Kleen <ak@...ux.intel.com>,
        Arnd Bergmann <arnd@...db.de>, Hoeun Ryu <hoeun.ryu@...il.com>,
        Hans-Christian Noren Egtvedt <egtvedt@...fundet.no>,
        linux-kernel@...r.kernel.org
Subject: [PATCH] uaccess: Zero destination buffer on overflow attempt

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 <viro@...iv.linux.org.uk>
Signed-off-by: Kees Cook <keescook@...omium.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