netdev - Re: [PATCH net-next 2/8] wifi: ath9k: silence array-bounds warning on GCC 12

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: <202205231229.CF6B8471@keescook>
Date:   Mon, 23 May 2022 12:31:23 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Kalle Valo <kvalo@...nel.org>
Cc:     Jakub Kicinski <kuba@...nel.org>, johannes@...solutions.net,
        netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
        toke@...e.dk, linux-kernel@...r.kernel.org,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [PATCH net-next 2/8] wifi: ath9k: silence array-bounds warning
 on GCC 12

On Sat, May 21, 2022 at 09:58:28AM +0300, Kalle Valo wrote:
> + arnd, kees, lkml
> 
> Jakub Kicinski <kuba@...nel.org> writes:
> 
> > GCC 12 says:
> >
> > drivers/net/wireless/ath/ath9k/mac.c: In function ‘ath9k_hw_resettxqueue’:
> > drivers/net/wireless/ath/ath9k/mac.c:373:22: warning: array subscript
> > 32 is above array bounds of ‘struct ath9k_tx_queue_info[10]’
> > [-Warray-bounds]
> >   373 |         qi = &ah->txq[q];
> >       |               ~~~~~~~^~~
> >
> > I don't know where it got the 32 from, relegate the warning to W=1+.
> >
> > Signed-off-by: Jakub Kicinski <kuba@...nel.org>
> > ---
> > CC: toke@...e.dk
> > CC: kvalo@...nel.org
> > CC: linux-wireless@...r.kernel.org
> > ---
> >  drivers/net/wireless/ath/ath9k/Makefile | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/Makefile b/drivers/net/wireless/ath/ath9k/Makefile
> > index eff94bcd1f0a..9bdfcee2f448 100644
> > --- a/drivers/net/wireless/ath/ath9k/Makefile
> > +++ b/drivers/net/wireless/ath/ath9k/Makefile
> > @@ -45,6 +45,11 @@ ath9k_hw-y:=	\
> >  		ar9003_eeprom.o \
> >  		ar9003_paprd.o
> >  
> > +# FIXME: temporarily silence -Warray-bounds on non W=1+ builds
> > +ifndef KBUILD_EXTRA_WARN
> > +CFLAGS_mac.o += -Wno-array-bounds
> > +endif
> 
> There are now four wireless drivers which need this hack. Wouldn't it be
> easier to add -Wno-array-bounds for GCC 12 globally instead of adding
> the same hack to multiple drivers?

I finally tracked this down to a GCC 12 bug related to -fsanitize=shift:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105679

Basically all the "32" stuff comes from the index being used in a shift,
and the resulting internal GCC logic blowing up.

I was going to do a before/after build with and without -fsanitize=shift
to see how many of these false positives originate from that bug...

-- 
Kees Cook