| 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
| ||
|
Date: Tue, 23 Aug 2016 14:28:27 -0500
From: Josh Poimboeuf <jpoimboe@...hat.com>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...nel.org>,
"H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
linux-kernel@...r.kernel.org,
Andy Lutomirski <luto@...capital.net>,
Steven Rostedt <rostedt@...dmis.org>,
Brian Gerst <brgerst@...il.com>,
Kees Cook <keescook@...omium.org>,
Peter Zijlstra <peterz@...radead.org>,
Frederic Weisbecker <fweisbec@...il.com>,
Byungchul Park <byungchul.park@....com>,
Nilay Vaish <nilayvaish@...il.com>
Subject: [PATCH 1/2] mm/usercopy: get rid of "provably correct" warnings
With CONFIG_DEBUG_STRICT_USER_COPY_CHECKS=y, if I force enable the
__compiletime_object_size() macro with a recent compiler by removing the
"GCC_VERSION < 40600" check, I get a bunch of false positive warnings.
For example:
In function ‘copy_to_user.part.8’,
inlined from ‘copy_to_user’,
inlined from ‘proc_put_long’ at /home/jpoimboe/git/linux/kernel/sysctl.c:2096:6:
/home/jpoimboe/git/linux/arch/x86/include/asm/uaccess.h:723:46: warning: call to ‘__copy_to_user_overflow’ declared with attribute warning: copy_to_user() buffer size is not provably correct
The problem is that gcc can't always definitively tell whether
copy_from_user_overflow() will be called. And when in doubt, it prints
the warning anyway. So in practice, these warnings mostly just create a
lot of noise. There might be a bug lurking in there somewhere, but the
signal to noise ratio is really low, and not worth the pain IMO.
So just remove the "provably correct" warnings altogether. This also
lays the groundwork for re-enabling the copy_from_user_overflow()
runtime warnings for newer compilers.
Signed-off-by: Josh Poimboeuf <jpoimboe@...hat.com>
---
arch/parisc/include/asm/uaccess.h | 8 +-------
arch/s390/include/asm/uaccess.h | 6 +-----
arch/tile/include/asm/uaccess.h | 3 +--
arch/x86/include/asm/uaccess.h | 35 -----------------------------------
4 files changed, 3 insertions(+), 49 deletions(-)
diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index 0f59fd9..b34c022 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -208,13 +208,7 @@ unsigned long copy_in_user(void __user *dst, const void __user *src, unsigned lo
#define __copy_to_user_inatomic __copy_to_user
#define __copy_from_user_inatomic __copy_from_user
-extern void copy_from_user_overflow(void)
-#ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
- __compiletime_error("copy_from_user() buffer size is not provably correct")
-#else
- __compiletime_warning("copy_from_user() buffer size is not provably correct")
-#endif
-;
+extern void copy_from_user_overflow(void);
static inline unsigned long __must_check copy_from_user(void *to,
const void __user *from,
diff --git a/arch/s390/include/asm/uaccess.h b/arch/s390/include/asm/uaccess.h
index 9b49cf1..6d36860 100644
--- a/arch/s390/include/asm/uaccess.h
+++ b/arch/s390/include/asm/uaccess.h
@@ -332,11 +332,7 @@ copy_to_user(void __user *to, const void *from, unsigned long n)
return __copy_to_user(to, from, n);
}
-void copy_from_user_overflow(void)
-#ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
-__compiletime_warning("copy_from_user() buffer size is not provably correct")
-#endif
-;
+void copy_from_user_overflow(void);
/**
* copy_from_user: - Copy a block of data from user space.
diff --git a/arch/tile/include/asm/uaccess.h b/arch/tile/include/asm/uaccess.h
index 0a9c4265..e0e313f 100644
--- a/arch/tile/include/asm/uaccess.h
+++ b/arch/tile/include/asm/uaccess.h
@@ -422,8 +422,7 @@ _copy_from_user(void *to, const void __user *from, unsigned long n)
* option is not really compatible with -Werror, which is more useful in
* general.
*/
-extern void copy_from_user_overflow(void)
- __compiletime_warning("copy_from_user() size is not provably correct");
+extern void copy_from_user_overflow(void);
static inline unsigned long __must_check copy_from_user(void *to,
const void __user *from,
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index a0ae610..89c12cb 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -710,20 +710,6 @@ copy_to_user_overflow(void) __asm__("copy_from_user_overflow");
#undef copy_user_diag
-#ifdef CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
-
-extern void
-__compiletime_warning("copy_from_user() buffer size is not provably correct")
-__copy_from_user_overflow(void) __asm__("copy_from_user_overflow");
-#define __copy_from_user_overflow(size, count) __copy_from_user_overflow()
-
-extern void
-__compiletime_warning("copy_to_user() buffer size is not provably correct")
-__copy_to_user_overflow(void) __asm__("copy_from_user_overflow");
-#define __copy_to_user_overflow(size, count) __copy_to_user_overflow()
-
-#else
-
static inline void
__copy_from_user_overflow(int size, unsigned long count)
{
@@ -732,8 +718,6 @@ __copy_from_user_overflow(int size, unsigned long count)
#define __copy_to_user_overflow __copy_from_user_overflow
-#endif
-
static inline unsigned long __must_check
copy_from_user(void *to, const void __user *from, unsigned long n)
{
@@ -743,24 +727,6 @@ copy_from_user(void *to, const void __user *from, unsigned long n)
kasan_check_write(to, n);
- /*
- * While we would like to have the compiler do the checking for us
- * even in the non-constant size case, any false positives there are
- * a problem (especially when DEBUG_STRICT_USER_COPY_CHECKS, but even
- * without - the [hopefully] dangerous looking nature of the warning
- * would make people go look at the respecitive call sites over and
- * over again just to find that there's no problem).
- *
- * And there are cases where it's just not realistic for the compiler
- * to prove the count to be in range. For example when multiple call
- * sites of a helper function - perhaps in different source files -
- * all doing proper range checking, yet the helper function not doing
- * so again.
- *
- * Therefore limit the compile time checking to the constant size
- * case, and do only runtime checking for non-constant sizes.
- */
-
if (likely(sz < 0 || sz >= n)) {
check_object_size(to, n, false);
n = _copy_from_user(to, from, n);
@@ -781,7 +747,6 @@ copy_to_user(void __user *to, const void *from, unsigned long n)
might_fault();
- /* See the comment in copy_from_user() above. */
if (likely(sz < 0 || sz >= n)) {
check_object_size(from, n, true);
n = _copy_to_user(to, from, n);
--
2.7.4
Powered by blists - more mailing lists