| 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
| ||
|
Message-ID: <194e2d22-9eff-ec6c-9bc9-13b62bda870a@roeck-us.net>
Date: Wed, 20 Sep 2023 07:30:22 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Rob Herring <robh@...nel.org>
Cc: kernel test robot <lkp@...el.com>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
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/20/23 05:44, Rob Herring wrote:
> On Tue, Sep 19, 2023 at 6:14 PM Guenter Roeck <linux@...ck-us.net> wrote:
>>
>> 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.
>
> Agreed. I keep getting one to fix "of of" in a comment. The fix is
> always to drop an "of", but that's actually wrong because it's
> supposed to be "OF". I keep pointing this out and *never* get the
> right fix. I don't fix it myself because at this point, I find
> continuing to get "fixes" entertaining.
>
>> I do wonder if enabling BUILD_TEST on such drivers is any better.
>
> I think that and trivial fixes on a specific driver are completely
> different. If you want to test build one driver, that's not too hard.
> Find its kconfig value, turn it on and build the right arch. For tree
> wide stuff, it's a real pain to ensure you built everything. For
> example, the only way to build many Cavium Octeon drivers is with the
> Octeon defconfig which is just one of dozens configs for MIPS (which
> is still a bigger mess than arm32 ever was).
>
Sure, but I still argue that this isn't worth it for drivers like this one.
Are you going to submit a fix ? Because otherwise I'll submit a patch
to drop COMPILE_TEST from MACHZ_WDT.
Thanks,
Guenter
Powered by blists - more mailing lists