[PATCH 1/2] mm/usercopy: get rid of "provably correct" warnings

[Josh Poimboeuf]

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@xxxxxxxxxx>
---
 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