linux-kernel - Re: [GIT PULL] pin control bulk changes for v5.18

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]

Date:   Mon, 28 Mar 2022 23:38:17 +0200
From:   Jonathan Neuschäfer <j.neuschaefer@....net>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Kees Cook <keescook@...omium.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>
Subject: Re: [GIT PULL] pin control bulk changes for v5.18

Hi,

On Mon, Mar 28, 2022 at 12:11:49PM -0700, Linus Torvalds wrote:
> On Mon, Mar 28, 2022 at 6:08 AM Linus Walleij <linus.walleij@...aro.org> wrote:
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-pinctrl.git
> > tags/pinctrl-v5.18-1
> 
> Hmm.
> 
> This clashes badly with the fact that we in the meantime have enabled
> -Warray-bounds, and I got
> 
>   drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c: In function ‘npcmgpio_irq_handler’:
>   ./include/linux/find.h:40:23: error: array subscript ‘long unsigned
> int[0]’ is partly outside array bounds of ‘u32[1]’ {aka ‘unsigned
> int[1]’} [-Werror=array-bounds]
>      40 |                 val = *addr & GENMASK(size - 1, offset);
>         |                       ^~~~~
>   drivers/pinctrl/nuvoton/pinctrl-npcm7xx.c:219:13: note: while
> referencing ‘sts’
>     219 |         u32 sts, en, bit;
>         |             ^~~
> 
> as a result.
> 
> Was this not in linux-next?

It was — but when it was noticed, the fix went through the IRQ tree, on
top of a refactoring that happened there (the switch to generic_handle_domain_irq):

https://lore.kernel.org/lkml/164751044707.389.16417510835118111853.tip-bot2@tip-bot2/

So… the issue should resolve itself when the IRQ tree is pulled.

> Or was the array bounds checking not there?
> 
> Anyway, that cast to "const void *"
> 
>      for_each_set_bit(bit, (const void *)&sts,
> 
> in that driver is completely wrong.
>
> The bit operations are defined in arrays of 'unsigned long', and you
> can't just cast the issue away, because the end result is not the same
> on a big-endian machine.
> 
> I fixed it up in the merge, but what really confuses me (apart from
> the apparent lack of testing in linux-next) is that I don't actually
> see what made this happen now, and not before. Maybe that's why it
> didn't show up in linux-next: it's some odd gcc heisenbug.
> 
> Because there seems to be no actual changes to that driver that would
> explain why I get the warning now, but not before the pull.

When I added the pinctrl-wpcm450 driver, I changed the pinctrl/nuvoton/
directory to obj-y and exposed the pinctrl-npcm7xx to CI bot testing.
The bug existed, untouched, since the driver was added a few years ago.


Jonathan

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)