[hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]

tree:   https://github.com/hnaz/linux-mm master
head:   a085ccd0a3e9741df5fe3a0e54228dc033759169
commit: e7c89f2e1992cc557c934608cc27e2b9759cb21a [169/698] string.h: fix incompatibility between FORTIFY_SOURCE and KASAN
config: arm-aspeed_g5_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        git checkout e7c89f2e1992cc557c934608cc27e2b9759cb21a
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <lkp@intel.com>

All warnings (new ones prefixed by >>, old ones prefixed by <<):

In file included from include/linux/bitmap.h:9,
from include/linux/cpumask.h:12,
from include/linux/rcupdate.h:31,
from include/linux/rculist.h:11,
from include/linux/pid.h:5,
from include/linux/sched.h:14,
from include/linux/ratelimit.h:6,
from include/linux/dev_printk.h:16,
from include/linux/device.h:15,
from include/linux/hdmi.h:28,
from drivers/video/hdmi.c:28:
In function 'strncpy',
inlined from 'hdmi_spd_infoframe_init' at drivers/video/hdmi.c:230:2:
include/linux/string.h:297:30: warning: '__builtin_strncpy' specified bound 8 equals destination size [-Wstringop-truncation]
297 | #define __underlying_strncpy __builtin_strncpy
|                              ^
>> include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
307 |  return __underlying_strncpy(p, q, size);
|         ^~~~~~~~~~~~~~~~~~~~
In function 'strncpy',
inlined from 'hdmi_spd_infoframe_init' at drivers/video/hdmi.c:231:2:
include/linux/string.h:297:30: warning: '__builtin_strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
297 | #define __underlying_strncpy __builtin_strncpy
|                              ^
>> include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
307 |  return __underlying_strncpy(p, q, size);
|         ^~~~~~~~~~~~~~~~~~~~

vim +/__underlying_strncpy +307 include/linux/string.h

   299	
   300	__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size)
   301	{
   302		size_t p_size = __builtin_object_size(p, 0);
   303		if (__builtin_constant_p(size) && p_size < size)
   304			__write_overflow();
   305		if (p_size < size)
   306			fortify_panic(__func__);
 > 307		return __underlying_strncpy(p, q, size);
   308	}
   309	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25926 bytes --]

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Andrew Morton]

On Sat, 30 May 2020 18:07:21 +0800 kbuild test robot <lkp@intel.com> wrote:

> tree:   https://github.com/hnaz/linux-mm master
> head:   a085ccd0a3e9741df5fe3a0e54228dc033759169
> commit: e7c89f2e1992cc557c934608cc27e2b9759cb21a [169/698] string.h: fix incompatibility between FORTIFY_SOURCE and KASAN
> config: arm-aspeed_g5_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         git checkout e7c89f2e1992cc557c934608cc27e2b9759cb21a
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm 
> 
> from include/linux/pid.h:5,
> from include/linux/sched.h:14,
> from include/linux/ratelimit.h:6,
> from include/linux/dev_printk.h:16,
> from include/linux/device.h:15,
> from include/linux/hdmi.h:28,
> from drivers/video/hdmi.c:28:
> In function 'strncpy',
> inlined from 'hdmi_spd_infoframe_init' at drivers/video/hdmi.c:230:2:
> include/linux/string.h:297:30: warning: '__builtin_strncpy' specified bound 8 equals destination size [-Wstringop-truncation]
> 297 | #define __underlying_strncpy __builtin_strncpy
> |                              ^
> >> include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
> 307 |  return __underlying_strncpy(p, q, size);
> |         ^~~~~~~~~~~~~~~~~~~~

Looks like another false positive.

Daniel, is there a practical way of suppressing these?

Thanks.

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Daniel Axtens]

Hi Andrew,

>> include/linux/string.h:297:30: warning: '__builtin_strncpy' specified bound 8 equals destination size [-Wstringop-truncation]
>> 297 | #define __underlying_strncpy __builtin_strncpy
>> |                              ^
>> >> include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
>> 307 |  return __underlying_strncpy(p, q, size);
>> |         ^~~~~~~~~~~~~~~~~~~~
>
> Looks like another false positive.
>
> Daniel, is there a practical way of suppressing these?

I think we probably want '-Wno-stringop-truncation'. That will stop
warnings where the bound equals the destination size (so there won't be
any null-truncation). But it won't stop the overflow warnings about
full-on overflows (-Wstringop-overflow).

I'll send a patch.

Regards,
Daniel

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Daniel Axtens]

Daniel Axtens <dja@axtens.net> writes:

> Hi Andrew,
>
>>> include/linux/string.h:297:30: warning: '__builtin_strncpy' specified bound 8 equals destination size [-Wstringop-truncation]
>>> 297 | #define __underlying_strncpy __builtin_strncpy
>>> |                              ^
>>> >> include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'
>>> 307 |  return __underlying_strncpy(p, q, size);
>>> |         ^~~~~~~~~~~~~~~~~~~~
>>
>> Looks like another false positive.
>>
>> Daniel, is there a practical way of suppressing these?
>
> I think we probably want '-Wno-stringop-truncation'. That will stop
> warnings where the bound equals the destination size (so there won't be
> any null-truncation). But it won't stop the overflow warnings about
> full-on overflows (-Wstringop-overflow).

Ah, so both stringop-truncate and stringop-overflow are disabled in
Makefile:
 - commit 5a76021c2eff ("gcc-10: disable 'stringop-overflow' warning for now")
 - commit 217c3e019675 ("disable stringop truncation warnings for now")

However, at W=1, stringop-truncate is re-enabled - last touched commit
64a91907c896 ("kbuild: refactor scripts/Makefile.extrawarn"). That would
explain why we see this on test builds with W=1 only.

I'll bump it to W=2.

> I'll send a patch.
>
> Regards,
> Daniel

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Daniel Axtens]

Huh, turns out that we do actually see a similar set of warnings before
and after this patch, they're just different warnings as a consequence
of my patch.

Given that these warnings only show up at W=1, is there any point in
further supressing them?

Before
------

In function ‘strncpy’,
    inlined from ‘hdmi_spd_infoframe_init’ at /home/dja/dev/linux/linux/drivers/video/hdmi.c:230:2:
/home/dja/dev/linux/linux/include/linux/string.h:282:9: warning: ‘__builtin_strncpy’ specified bound 8 equals destination size [-Wstringop-truncation]
  282 |  return __builtin_strncpy(p, q, size);
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘hdmi_spd_infoframe_init’ at /home/dja/dev/linux/linux/drivers/video/hdmi.c:231:2:
/home/dja/dev/linux/linux/include/linux/string.h:282:9: warning: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Wstringop-truncation]
  282 |  return __builtin_strncpy(p, q, size);


After
-----

In function ‘strncpy’,
    inlined from ‘hdmi_spd_infoframe_init’ at /home/dja/dev/linux/linux/drivers/video/hdmi.c:230:2:
/home/dja/dev/linux/linux/include/linux/string.h:297:30: warning: ‘__builtin_strncpy’ specified bound 8 equals destination size [-Wstringop-truncation]
  297 | #define __underlying_strncpy __builtin_strncpy
      |                              ^
/home/dja/dev/linux/linux/include/linux/string.h:307:9: note: in expansion of macro ‘__underlying_strncpy’
  307 |  return __underlying_strncpy(p, q, size);
      |         ^~~~~~~~~~~~~~~~~~~~
In function ‘strncpy’,
    inlined from ‘hdmi_spd_infoframe_init’ at /home/dja/dev/linux/linux/drivers/video/hdmi.c:231:2:
/home/dja/dev/linux/linux/include/linux/string.h:297:30: warning: ‘__builtin_strncpy’ specified bound 16 equals destination size [-Wstringop-truncation]
  297 | #define __underlying_strncpy __builtin_strncpy
      |                              ^
/home/dja/dev/linux/linux/include/linux/string.h:307:9: note: in expansion of macro ‘__underlying_strncpy’
  307 |  return __underlying_strncpy(p, q, size);
      |         ^~~~~~~~~~~~~~~~~~~~

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Andrew Morton]

On Tue, 02 Jun 2020 15:55:27 +1000 Daniel Axtens <dja@axtens.net> wrote:

> Huh, turns out that we do actually see a similar set of warnings before
> and after this patch, they're just different warnings as a consequence
> of my patch.

Ah.
 
> Given that these warnings only show up at W=1, is there any point in
> further supressing them?

I guess not.

So what should we do?  Go over the various sites and use memcpy(), each
with a suitably apologetic code comment explaining the reason?

Re: [hnaz-linux-mm:master 169/698] include/linux/string.h:307:9: note: in expansion of macro '__underlying_strncpy'

[Daniel Axtens]

Andrew Morton <akpm@linux-foundation.org> writes:

> On Tue, 02 Jun 2020 15:55:27 +1000 Daniel Axtens <dja@axtens.net> wrote:
>
>> Huh, turns out that we do actually see a similar set of warnings before
>> and after this patch, they're just different warnings as a consequence
>> of my patch.
>
> Ah.
>  
>> Given that these warnings only show up at W=1, is there any point in
>> further supressing them?
>
> I guess not.
>
> So what should we do?  Go over the various sites and use memcpy(), each
> with a suitably apologetic code comment explaining the reason?

It's a good question.

 - We could disable the warning outright or demote it to W=2+. It
   doesn't tell us about behaviour that is definitely wrong, just
   suspicious, and we have a number of callsites in the kenel using it
   in a correct and compliant way. Wstringop-overflow catches
   full-on overflows (once Linus reenables it).

 - We could keep the warning and patch the callsites. One can make the
   argument that changing to memcpy makes the semantics more clear: str*
   in C refers to null-terminated strings, and if the string isn't
   null-terminated then the str* functions aren't a natural fit for
   manipulating it. And it means we get to keep the warning enabled to
   catch accidental off-by-ones.

 - We could do nothing. If you compile with W=1 there is a known set of
   warnings and you're probably more interested in the delta than in the
   set.

I think I have a slight preference for patching the callsites and
eventually promoting the warning from W=1 to always on. But I have to be
honest that I'm not going to have time to do it soon, and potentially
not for a few months.

Regards,
Daniel