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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a57afd40-71a2-aa68-84cb-44d2a88a1e2b@roeck-us.net>
Date:   Tue, 19 Sep 2023 16:14:04 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Rob Herring <robh@...nel.org>, kernel test robot <lkp@...el.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     oe-kbuild-all@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Wim Van Sebroeck <wim@...ux-watchdog.org>
Subject: Re: arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0
 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned
 int[]'}

On 9/19/23 11:37, Rob Herring wrote:
> On Tue, Sep 19, 2023 at 7:09 AM kernel test robot <lkp@...el.com> wrote:
>>
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
>> head:   2cf0f715623872823a72e451243bbf555d10d032
>> commit: f1a43aadb5a690e141a3b6700e2a40c1d4dbe088 watchdog: Enable COMPILE_TEST for more drivers
>> date:   5 weeks ago
>> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230919/202309192013.vI4DKHmw-lkp@intel.com/config)
>> compiler: m68k-linux-gcc (GCC) 13.2.0
>> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230919/202309192013.vI4DKHmw-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@...el.com>
>> | Closes: https://lore.kernel.org/oe-kbuild-all/202309192013.vI4DKHmw-lkp@intel.com/
>>
>> All warnings (new ones prefixed by >>):
>>
>>     In file included from arch/m68k/include/asm/io_mm.h:25,
>>                      from arch/m68k/include/asm/io.h:8,
>>                      from include/linux/io.h:13,
>>                      from drivers/watchdog/machzwd.c:39:
>>     In function 'zf_set_timer',
>>         inlined from 'zf_timer_on' at drivers/watchdog/machzwd.c:218:2:
>>>> arch/m68k/include/asm/raw_io.h:91:13: warning: array subscript 0 is outside array bounds of 'volatile u16[0]' {aka 'volatile short unsigned int[]'} [-Warray-bounds=]
>>        91 |         __w = ((*(__force volatile u16 *) ((_addr & 0xFFFF0000UL) + ((__v >> 8)<<1)))); \
>>           |         ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>     arch/m68k/include/asm/io_mm.h:228:20: note: in expansion of macro 'rom_out_le16'
>>       228 |                  : rom_out_le16(isa_itw(port), (val)))
>>           |                    ^~~~~~~~~~~~
>>     arch/m68k/include/asm/io_mm.h:356:42: note: in expansion of macro 'isa_rom_outw'
>>       356 | #define outw(val, port) ((port) < 1024 ? isa_rom_outw((val), (port)) : out_le16((port), (val)))
>>           |                                          ^~~~~~~~~~~~
>>     drivers/watchdog/machzwd.c:74:53: note: in expansion of macro 'outw'
>>        74 | #define zf_writew(port, data)  { outb(port, INDEX); outw(data, DATA_W); }
>>           |                                                     ^~~~
>>     drivers/watchdog/machzwd.c:173:17: note: in expansion of macro 'zf_writew'
>>       173 |                 zf_writew(COUNTER_1, new);
>>           |                 ^~~~~~~~~
>>     In function 'zf_timer_on':
>>     cc1: note: source object is likely at address zero
> 
> This seems to be some newish check in gcc which looks for fixed
> pointers below 4KB[1]. The linked issue says more was planned for
> gcc-13, but I haven't found what that is. AFAICT, that shouldn't
> happen with this config because isa_itw() should be variable and the
> compiler shouldn't be able to determine the value of _addr. However, a
> config with CONFIG_Q40=n, CONFIG_AMIGA_PCMCIA=n, and
> CONFIG_ATARI_ROM_ISA=n would have a fixed NULL value and could trigger
> the warning. This should also have warnings everywhere outw() (and
> others) are used with a constant port value.
> 
> Rob
> 
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578

A long time ago, when someone submitted a "cleanup: patch for the machzwd
watchdog driver, I approved it but added this comment.

 > If anyone is still using the HW supported by this driver, it would
 > be a prime target for conversion to use the watchdog subsystem.
 > This would reduce code size and improve driver stability.
 > _That_ would be useful.

 > The only reason for replacing 0 with NULL is to make a static checker
 > happy. This kind of change adds zero value to the code. Instead, it
 > takes time from those who have to review the patches and apply them.

 > If we ignore such patches, we'll get them again and again, taking
 > away even more time. If we don't ignore them, we encourage submitters
 > and get even more useless patches. If we try to discourage them, we
 > risk being accused of being unfriendly. This is a perfect lose-lose
 > situation for maintainers.

I do wonder if enabling BUILD_TEST on such drivers is any better.

Guenter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ