diff mbox series

copy_{to,from}_user(): fix compile-time sanity checks with gcc -Og

Message ID 20181102155710.2299475-1-arnd@arndb.de
State New
Headers show
Series copy_{to,from}_user(): fix compile-time sanity checks with gcc -Og | expand

Commit Message

Arnd Bergmann Nov. 2, 2018, 3:56 p.m. UTC
When CONFIG_CC_OPTIMIZE_FOR_DEBUGGING is set, we get countless warnings
like

In function 'check_copy_size',
    inlined from 'copy_from_user' at include/linux/uaccess.h:146:6,
    inlined from '__copy_siginfo_from_user' at kernel/signal.c:3032:6:
include/linux/thread_info.h:147:4: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

It seems that constant propagation doesn't work well enough to make
this code reliable, so turn it off for that configuration.

Cc: Changbin Du<changbin.du@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 include/linux/thread_info.h | 5 +++++
 1 file changed, 5 insertions(+)

-- 
2.18.0

Comments

Changbin Du Nov. 3, 2018, 12:03 a.m. UTC | #1
On Fri, Nov 02, 2018 at 04:56:58PM +0100, Arnd Bergmann wrote:
> When CONFIG_CC_OPTIMIZE_FOR_DEBUGGING is set, we get countless warnings

> like

> 

> In function 'check_copy_size',

>     inlined from 'copy_from_user' at include/linux/uaccess.h:146:6,

>     inlined from '__copy_siginfo_from_user' at kernel/signal.c:3032:6:

> include/linux/thread_info.h:147:4: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

> 

> It seems that constant propagation doesn't work well enough to make

> this code reliable, so turn it off for that configuration.

> 

This is caused by __compiletime_warning() and fixed by below change
already. Could you try the latest kbuild tree?

--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -86,8 +86,10 @@
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)

 #ifndef __CHECKER__
+#ifndef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
+#endif

> Cc: Changbin Du<changbin.du@gmail.com>

> Cc: Al Viro <viro@zeniv.linux.org.uk>

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  include/linux/thread_info.h | 5 +++++

>  1 file changed, 5 insertions(+)

> 

> diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h

> index 8d8821b3689a..762f379bdf5d 100644

> --- a/include/linux/thread_info.h

> +++ b/include/linux/thread_info.h

> @@ -138,6 +138,11 @@ static __always_inline bool

>  check_copy_size(const void *addr, size_t bytes, bool is_source)

>  {

>  	int sz = __compiletime_object_size(addr);

> +

> +	/* constant propagation doesn't work well with -Og */

> +	if (IS_ENABLED(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING))

> +		return true;

> +

>  	if (unlikely(sz >= 0 && sz < bytes)) {

>  		if (!__builtin_constant_p(bytes))

>  			copy_overflow(sz, bytes);

> -- 

> 2.18.0

> 


-- 
Thanks,
Changbin Du
Arnd Bergmann Nov. 3, 2018, 3:46 p.m. UTC | #2
On 11/3/18, Changbin Du <changbin.du@gmail.com> wrote:
> On Fri, Nov 02, 2018 at 04:56:58PM +0100, Arnd Bergmann wrote:

>> When CONFIG_CC_OPTIMIZE_FOR_DEBUGGING is set, we get countless warnings

>> like

>>

>> In function 'check_copy_size',

>>     inlined from 'copy_from_user' at include/linux/uaccess.h:146:6,

>>     inlined from '__copy_siginfo_from_user' at kernel/signal.c:3032:6:

>> include/linux/thread_info.h:147:4: error: call to '__bad_copy_to' declared

>> with attribute error: copy destination size is too small

>>

>> It seems that constant propagation doesn't work well enough to make

>> this code reliable, so turn it off for that configuration.

>>

> This is caused by __compiletime_warning() and fixed by below change

> already. Could you try the latest kbuild tree?

>

> --- a/include/linux/compiler-gcc.h

> +++ b/include/linux/compiler-gcc.h

> @@ -86,8 +86,10 @@

>  #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)

>

>  #ifndef __CHECKER__

> +#ifndef CONFIG_CC_OPTIMIZE_FOR_DEBUGGING

>  #define __compiletime_warning(message) __attribute__((warning(message)))

>  #define __compiletime_error(message) __attribute__((error(message)))

> +#endif


Right, that works as well, but my version is a bit less invasive as it
only disables the __compiletime_error in the one file that introduced
the regression, and not all of them. I have built hundreds of randconfig
builds and found no other problem with __compiletime_error(), though
I did find a bunch of (correct) section mismatch warnings and
false-positive -Wformat-overflow= warnings with
CONFIG_CC_OPTIMIZE_FOR_DEBUGGING (patches pending).

I also got some link errors for which I already posted patches.

       Arnd
diff mbox series

Patch

diff --git a/include/linux/thread_info.h b/include/linux/thread_info.h
index 8d8821b3689a..762f379bdf5d 100644
--- a/include/linux/thread_info.h
+++ b/include/linux/thread_info.h
@@ -138,6 +138,11 @@  static __always_inline bool
 check_copy_size(const void *addr, size_t bytes, bool is_source)
 {
 	int sz = __compiletime_object_size(addr);
+
+	/* constant propagation doesn't work well with -Og */
+	if (IS_ENABLED(CONFIG_CC_OPTIMIZE_FOR_DEBUGGING))
+		return true;
+
 	if (unlikely(sz >= 0 && sz < bytes)) {
 		if (!__builtin_constant_p(bytes))
 			copy_overflow(sz, bytes);