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

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

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   bb066fe812d6fb3a9d01c073d9f1e2fd5a63403b
commit: c301e1fefc2d6c83a2beb47e9cdd7b59a90b0067 ntsync: No longer depend on BROKEN.
date:   4 weeks ago
config: i386-randconfig-001-20250207 (https://download.01.org/0day-ci/archive/20250207/202502072019.LYoCR9bF-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250207/202502072019.LYoCR9bF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502072019.LYoCR9bF-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/spinlock.h:60,
                    from include/linux/wait.h:9,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from drivers/misc/ntsync.c:11:
   In function 'check_copy_size',
       inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
       inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
>> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
     259 |                         __bad_copy_to();
         |                         ^~~~~~~~~~~~~~~


vim +/__bad_copy_to +259 include/linux/thread_info.h

b0377fedb652808 Al Viro   2017-06-29  248  
9dd819a15162f8f Kees Cook 2019-09-25  249  static __always_inline __must_check bool
b0377fedb652808 Al Viro   2017-06-29  250  check_copy_size(const void *addr, size_t bytes, bool is_source)
b0377fedb652808 Al Viro   2017-06-29  251  {
c80d92fbb67b2c8 Kees Cook 2021-06-17  252  	int sz = __builtin_object_size(addr, 0);
b0377fedb652808 Al Viro   2017-06-29  253  	if (unlikely(sz >= 0 && sz < bytes)) {
b0377fedb652808 Al Viro   2017-06-29  254  		if (!__builtin_constant_p(bytes))
b0377fedb652808 Al Viro   2017-06-29  255  			copy_overflow(sz, bytes);
b0377fedb652808 Al Viro   2017-06-29  256  		else if (is_source)
b0377fedb652808 Al Viro   2017-06-29  257  			__bad_copy_from();
b0377fedb652808 Al Viro   2017-06-29  258  		else
b0377fedb652808 Al Viro   2017-06-29 @259  			__bad_copy_to();
b0377fedb652808 Al Viro   2017-06-29  260  		return false;
b0377fedb652808 Al Viro   2017-06-29  261  	}
6d13de1489b6bf5 Kees Cook 2019-12-04  262  	if (WARN_ON_ONCE(bytes > INT_MAX))
6d13de1489b6bf5 Kees Cook 2019-12-04  263  		return false;
b0377fedb652808 Al Viro   2017-06-29  264  	check_object_size(addr, bytes, is_source);
b0377fedb652808 Al Viro   2017-06-29  265  	return true;
b0377fedb652808 Al Viro   2017-06-29  266  }
b0377fedb652808 Al Viro   2017-06-29  267  

:::::: The code at line 259 was first introduced by commit
:::::: b0377fedb6528087ed319b0d054d6ed82240372c copy_{to,from}_user(): consolidate object size checks

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

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Elizabeth Figura]

On Friday, 7 February 2025 06:11:47 CST kernel test robot wrote:
>    In file included from include/linux/spinlock.h:60,
>                     from include/linux/wait.h:9,
>                     from include/linux/wait_bit.h:8,
>                     from include/linux/fs.h:6,
>                     from drivers/misc/ntsync.c:11:
>    In function 'check_copy_size',
>        inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
>        inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
> >> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
>      259 |                         __bad_copy_to();
>          |                         ^~~~~~~~~~~~~~~

This was caught before and mentioned in [1]. The suggestion there of changing "args->count" to "count" doesn't help.

Somehow gcc 12 thinks that the array_size(count, sizeof(*fds)) parameter is constant, although it's finnicky and depends on exactly where __builtin_constant_p() is evaluated.

The bug goes away with gcc 13. Is this worth trying to work around? I don't have any ideas for how to do so.

[1] https://lore.kernel.org/all/21811752-06d3-44cd-b3e6-f8124676df87@app.fastmail.com/

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Philip Li]

On Mon, Feb 10, 2025 at 02:39:46PM -0600, Elizabeth Figura wrote:
> On Friday, 7 February 2025 06:11:47 CST kernel test robot wrote:
> >    In file included from include/linux/spinlock.h:60,
> >                     from include/linux/wait.h:9,
> >                     from include/linux/wait_bit.h:8,
> >                     from include/linux/fs.h:6,
> >                     from drivers/misc/ntsync.c:11:
> >    In function 'check_copy_size',
> >        inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
> >        inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
> > >> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
> >      259 |                         __bad_copy_to();
> >          |                         ^~~~~~~~~~~~~~~
> 
> This was caught before and mentioned in [1]. The suggestion there of changing "args->count" to "count" doesn't help.
> 
> Somehow gcc 12 thinks that the array_size(count, sizeof(*fds)) parameter is constant, although it's finnicky and depends on exactly where __builtin_constant_p() is evaluated.
> 
> The bug goes away with gcc 13. Is this worth trying to work around? I don't have any ideas for how to do so.

Thanks for the info, at bot side, we will ignore this error to
avoid further reporting on old gcc.

> 
> [1] https://lore.kernel.org/all/21811752-06d3-44cd-b3e6-f8124676df87@app.fastmail.com/
> 
> 

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Arnd Bergmann]

On Fri, Feb 14, 2025, at 09:44, Philip Li wrote:
> On Mon, Feb 10, 2025 at 02:39:46PM -0600, Elizabeth Figura wrote:
>> On Friday, 7 February 2025 06:11:47 CST kernel test robot wrote:
>> >    In file included from include/linux/spinlock.h:60,
>> >                     from include/linux/wait.h:9,
>> >                     from include/linux/wait_bit.h:8,
>> >                     from include/linux/fs.h:6,
>> >                     from drivers/misc/ntsync.c:11:
>> >    In function 'check_copy_size',
>> >        inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
>> >        inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
>> > >> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
>> >      259 |                         __bad_copy_to();
>> >          |                         ^~~~~~~~~~~~~~~
>> 
>> This was caught before and mentioned in [1]. The suggestion there of changing "args->count" to "count" doesn't help.
>> 
>> Somehow gcc 12 thinks that the array_size(count, sizeof(*fds)) parameter is constant, although it's finnicky and depends on exactly where __builtin_constant_p() is evaluated.
>> 
>> The bug goes away with gcc 13. Is this worth trying to work around? I don't have any ideas for how to do so.
>
> Thanks for the info, at bot side, we will ignore this error to
> avoid further reporting on old gcc.

gcc-12 isn't really "old", we support gcc-5 through gcc-14 at the moment.

Maybe the change below would address it? (Not sure I handled the
"+1" right here, but something like that should make it obvious
to the compiler what the check is).

      Arnd

diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
index 055395cde42b..ae13aae9194b 100644
--- a/drivers/misc/ntsync.c
+++ b/drivers/misc/ntsync.c
@@ -873,6 +873,7 @@ static int setup_wait(struct ntsync_device *dev,
 {
 	int fds[NTSYNC_MAX_WAIT_COUNT + 1];
 	const __u32 count = args->count;
+	size_t size = count * sizeof(fds[0]);
 	struct ntsync_q *q;
 	__u32 total_count;
 	__u32 i, j;
@@ -880,15 +881,14 @@ static int setup_wait(struct ntsync_device *dev,
 	if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME))
 		return -EINVAL;
 
-	if (args->count > NTSYNC_MAX_WAIT_COUNT)
+	if (size >= sizeof(fds))
 		return -EINVAL;
 
 	total_count = count;
 	if (args->alert)
 		total_count++;
 
-	if (copy_from_user(fds, u64_to_user_ptr(args->objs),
-			   array_size(count, sizeof(*fds))))
+	if (copy_from_user(fds, u64_to_user_ptr(args->objs), size))
 		return -EFAULT;
 	if (args->alert)
 		fds[count] = args->alert;

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Oliver Sang]

hi, Arnd,

On Fri, Feb 14, 2025 at 12:22:33PM +0100, Arnd Bergmann wrote:
> 
> 
> On Fri, Feb 14, 2025, at 09:44, Philip Li wrote:
> > On Mon, Feb 10, 2025 at 02:39:46PM -0600, Elizabeth Figura wrote:
> >> On Friday, 7 February 2025 06:11:47 CST kernel test robot wrote:
> >> >    In file included from include/linux/spinlock.h:60,
> >> >                     from include/linux/wait.h:9,
> >> >                     from include/linux/wait_bit.h:8,
> >> >                     from include/linux/fs.h:6,
> >> >                     from drivers/misc/ntsync.c:11:
> >> >    In function 'check_copy_size',
> >> >        inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
> >> >        inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
> >> > >> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
> >> >      259 |                         __bad_copy_to();
> >> >          |                         ^~~~~~~~~~~~~~~
> >> 
> >> This was caught before and mentioned in [1]. The suggestion there of changing "args->count" to "count" doesn't help.
> >> 
> >> Somehow gcc 12 thinks that the array_size(count, sizeof(*fds)) parameter is constant, although it's finnicky and depends on exactly where __builtin_constant_p() is evaluated.
> >> 
> >> The bug goes away with gcc 13. Is this worth trying to work around? I don't have any ideas for how to do so.
> >
> > Thanks for the info, at bot side, we will ignore this error to
> > avoid further reporting on old gcc.
> 
> gcc-12 isn't really "old", we support gcc-5 through gcc-14 at the moment.
> 
> Maybe the change below would address it? (Not sure I handled the
> "+1" right here, but something like that should make it obvious
> to the compiler what the check is).

yes, below change fixed the build error we reported. thanks

Tested-by: kernel test robot <oliver.sang@intel.com>

> 
>       Arnd
> 
> diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> index 055395cde42b..ae13aae9194b 100644
> --- a/drivers/misc/ntsync.c
> +++ b/drivers/misc/ntsync.c
> @@ -873,6 +873,7 @@ static int setup_wait(struct ntsync_device *dev,
>  {
>  	int fds[NTSYNC_MAX_WAIT_COUNT + 1];
>  	const __u32 count = args->count;
> +	size_t size = count * sizeof(fds[0]);
>  	struct ntsync_q *q;
>  	__u32 total_count;
>  	__u32 i, j;
> @@ -880,15 +881,14 @@ static int setup_wait(struct ntsync_device *dev,
>  	if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME))
>  		return -EINVAL;
>  
> -	if (args->count > NTSYNC_MAX_WAIT_COUNT)
> +	if (size >= sizeof(fds))
>  		return -EINVAL;
>  
>  	total_count = count;
>  	if (args->alert)
>  		total_count++;
>  
> -	if (copy_from_user(fds, u64_to_user_ptr(args->objs),
> -			   array_size(count, sizeof(*fds))))
> +	if (copy_from_user(fds, u64_to_user_ptr(args->objs), size))
>  		return -EFAULT;
>  	if (args->alert)
>  		fds[count] = args->alert;

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Arnd Bergmann]

On Tue, Feb 18, 2025, at 03:44, Oliver Sang wrote:
> On Fri, Feb 14, 2025 at 12:22:33PM +0100, Arnd Bergmann wrote:
>> On Fri, Feb 14, 2025, at 09:44, Philip Li wrote:
>> > On Mon, Feb 10, 2025 at 02:39:46PM -0600, Elizabeth Figura wrote:
>> 
>> Maybe the change below would address it? (Not sure I handled the
>> "+1" right here, but something like that should make it obvious
>> to the compiler what the check is).
>
> yes, below change fixed the build error we reported. thanks
>
> Tested-by: kernel test robot <oliver.sang@intel.com>

Thanks for testing!

Elizabeth, can you turn this into a proper patch with
"Suggested-by: Arnd Bergmann <arnd@arndb.de>"? I assume
you'll want to make some slight changes to ensure it's
both readable and correct, but apparently calculating
the size in bytes and do both the range checking and
the copy based on that is what makes gcc see the limit
correctly.

      Arnd

Re: include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small

[Geert Uytterhoeven]

Hi Arnd,

On Fri, 14 Feb 2025 at 12:26, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Feb 14, 2025, at 09:44, Philip Li wrote:
> > On Mon, Feb 10, 2025 at 02:39:46PM -0600, Elizabeth Figura wrote:
> >> On Friday, 7 February 2025 06:11:47 CST kernel test robot wrote:
> >> >    In file included from include/linux/spinlock.h:60,
> >> >                     from include/linux/wait.h:9,
> >> >                     from include/linux/wait_bit.h:8,
> >> >                     from include/linux/fs.h:6,
> >> >                     from drivers/misc/ntsync.c:11:
> >> >    In function 'check_copy_size',
> >> >        inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
> >> >        inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
> >> > >> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
> >> >      259 |                         __bad_copy_to();
> >> >          |                         ^~~~~~~~~~~~~~~
> >>
> >> This was caught before and mentioned in [1]. The suggestion there of changing "args->count" to "count" doesn't help.
> >>
> >> Somehow gcc 12 thinks that the array_size(count, sizeof(*fds)) parameter is constant, although it's finnicky and depends on exactly where __builtin_constant_p() is evaluated.
> >>
> >> The bug goes away with gcc 13. Is this worth trying to work around? I don't have any ideas for how to do so.
> >
> > Thanks for the info, at bot side, we will ignore this error to
> > avoid further reporting on old gcc.
>
> gcc-12 isn't really "old", we support gcc-5 through gcc-14 at the moment.
>
> Maybe the change below would address it? (Not sure I handled the
> "+1" right here, but something like that should make it obvious
> to the compiler what the check is).
>
>       Arnd
>
> diff --git a/drivers/misc/ntsync.c b/drivers/misc/ntsync.c
> index 055395cde42b..ae13aae9194b 100644
> --- a/drivers/misc/ntsync.c
> +++ b/drivers/misc/ntsync.c
> @@ -873,6 +873,7 @@ static int setup_wait(struct ntsync_device *dev,
>  {
>         int fds[NTSYNC_MAX_WAIT_COUNT + 1];
>         const __u32 count = args->count;
> +       size_t size = count * sizeof(fds[0]);

array_size(), to keep the multiplication overflow check?

>         struct ntsync_q *q;
>         __u32 total_count;
>         __u32 i, j;
> @@ -880,15 +881,14 @@ static int setup_wait(struct ntsync_device *dev,
>         if (args->pad || (args->flags & ~NTSYNC_WAIT_REALTIME))
>                 return -EINVAL;
>
> -       if (args->count > NTSYNC_MAX_WAIT_COUNT)
> +       if (size >= sizeof(fds))
>                 return -EINVAL;
>
>         total_count = count;
>         if (args->alert)
>                 total_count++;
>
> -       if (copy_from_user(fds, u64_to_user_ptr(args->objs),
> -                          array_size(count, sizeof(*fds))))
> +       if (copy_from_user(fds, u64_to_user_ptr(args->objs), size))
>                 return -EFAULT;
>         if (args->alert)
>                 fds[count] = args->alert;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

[kernel test robot]

Hi Elizabeth,

FYI, the error/warning still remains.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head:   5cf80612d3f72c46ad53ef5042b4c609c393122f
commit: c301e1fefc2d6c83a2beb47e9cdd7b59a90b0067 ntsync: No longer depend on BROKEN.
date:   6 weeks ago
config: i386-buildonly-randconfig-002-20250221 (https://download.01.org/0day-ci/archive/20250223/202502230554.IA0tyUeS-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250223/202502230554.IA0tyUeS-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202502230554.IA0tyUeS-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from include/linux/spinlock.h:60,
                    from include/linux/wait.h:9,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from drivers/misc/ntsync.c:11:
   In function 'check_copy_size',
       inlined from 'copy_from_user' at include/linux/uaccess.h:207:7,
       inlined from 'setup_wait' at drivers/misc/ntsync.c:888:6:
>> include/linux/thread_info.h:259:25: error: call to '__bad_copy_to' declared with attribute error: copy destination size is too small
     259 |                         __bad_copy_to();
         |                         ^~~~~~~~~~~~~~~


vim +/__bad_copy_to +259 include/linux/thread_info.h

b0377fedb65280 Al Viro   2017-06-29  248  
9dd819a15162f8 Kees Cook 2019-09-25  249  static __always_inline __must_check bool
b0377fedb65280 Al Viro   2017-06-29  250  check_copy_size(const void *addr, size_t bytes, bool is_source)
b0377fedb65280 Al Viro   2017-06-29  251  {
c80d92fbb67b2c Kees Cook 2021-06-17  252  	int sz = __builtin_object_size(addr, 0);
b0377fedb65280 Al Viro   2017-06-29  253  	if (unlikely(sz >= 0 && sz < bytes)) {
b0377fedb65280 Al Viro   2017-06-29  254  		if (!__builtin_constant_p(bytes))
b0377fedb65280 Al Viro   2017-06-29  255  			copy_overflow(sz, bytes);
b0377fedb65280 Al Viro   2017-06-29  256  		else if (is_source)
b0377fedb65280 Al Viro   2017-06-29  257  			__bad_copy_from();
b0377fedb65280 Al Viro   2017-06-29  258  		else
b0377fedb65280 Al Viro   2017-06-29 @259  			__bad_copy_to();
b0377fedb65280 Al Viro   2017-06-29  260  		return false;
b0377fedb65280 Al Viro   2017-06-29  261  	}
6d13de1489b6bf Kees Cook 2019-12-04  262  	if (WARN_ON_ONCE(bytes > INT_MAX))
6d13de1489b6bf Kees Cook 2019-12-04  263  		return false;
b0377fedb65280 Al Viro   2017-06-29  264  	check_object_size(addr, bytes, is_source);
b0377fedb65280 Al Viro   2017-06-29  265  	return true;
b0377fedb65280 Al Viro   2017-06-29  266  }
b0377fedb65280 Al Viro   2017-06-29  267  

:::::: The code at line 259 was first introduced by commit
:::::: b0377fedb6528087ed319b0d054d6ed82240372c copy_{to,from}_user(): consolidate object size checks

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

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki