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

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

[Kalle Valo]

+ arnd, kees, lkml

Jakub Kicinski <kuba@kernel.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@kernel.org>
> ---
> CC: toke@toke.dk
> CC: kvalo@kernel.org
> CC: linux-wireless@vger.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?

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

[Jakub Kicinski]

On Sat, 21 May 2022 09:58:28 +0300 Kalle Valo wrote:
> > +# 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 mean.. it's definitely a hack, I'm surprised more people aren't
complaining. Kees was against disabling it everywhere, AFAIU:

https://lore.kernel.org/all/202204201117.F44DCF9@keescook/

WiFi is a bit unfortunate but we only have 3 cases in the rest of
networking so it's not _terribly_ common.

IDK, I'd love to not see all the warnings every time someone touches
netdevice.h :( I made a note to remove the workaround once GCC 12 gets
its act together, that's the best I could come up with.

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

[Kalle Valo]

Jakub Kicinski <kuba@kernel.org> writes:

> On Sat, 21 May 2022 09:58:28 +0300 Kalle Valo wrote:
>> > +# 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 mean.. it's definitely a hack, I'm surprised more people aren't
> complaining. Kees was against disabling it everywhere, AFAIU:
>
> https://lore.kernel.org/all/202204201117.F44DCF9@keescook/

Wasn't Kees objecting of disabling array-bounds for all GCC versions?
That I understand, but I'm merely suggesting to disable the warning only
on GCC 12 until the compiler is fixed or the drivers are fixed.

> WiFi is a bit unfortunate but we only have 3 cases in the rest of
> networking so it's not _terribly_ common.
>
> IDK, I'd love to not see all the warnings every time someone touches
> netdevice.h :( I made a note to remove the workaround once GCC 12 gets
> its act together, that's the best I could come up with.

Ok, fair enough. I'm just worried these will be left lingering for a
long time and do more harm than good :)

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

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

[Kees Cook]

On Sat, May 21, 2022 at 09:58:28AM +0300, Kalle Valo wrote:
> + arnd, kees, lkml
> 
> Jakub Kicinski <kuba@kernel.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@kernel.org>
> > ---
> > CC: toke@toke.dk
> > CC: kvalo@kernel.org
> > CC: linux-wireless@vger.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