[PATCH net-next 0/8] Fix/silence GCC 12 warnings in drivers/net/wireless/

[PATCH net-next 0/8] Fix/silence GCC 12 warnings in drivers/net/wireless/

[Jakub Kicinski]

Hi Kalle & Johannes,

as mentioned off list we'd like to get GCC 12 warnings quashed.
This set takes care of the warnings we have in drivers/net/wireless/
mostly by relegating them to W=1/W=2 builds.

Is it okay for us to take this directly to net-next?
Or perhaps via wireless-next with a quick PR by Monday?

Jakub Kicinski (8):
  wifi: plfxlc: remove redundant NULL-check for GCC 12
  wifi: ath9k: silence array-bounds warning on GCC 12
  wifi: rtlwifi: remove always-true condition pointed out by GCC 12
  wifi: ath6k: silence false positive -Wno-dangling-pointer warning on
    GCC 12
  wifi: iwlwifi: use unsigned to silence a GCC 12 warning
  wifi: brcmfmac: work around a GCC 12 -Warray-bounds warning
  wifi: libertas: silence a GCC 12 -Warray-bounds warning
  wifi: carl9170: silence a GCC 12 -Warray-bounds warning

 drivers/net/wireless/ath/ath6kl/Makefile                    | 5 +++++
 drivers/net/wireless/ath/ath9k/Makefile                     | 5 +++++
 drivers/net/wireless/ath/carl9170/Makefile                  | 5 +++++
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c                | 2 +-
 drivers/net/wireless/marvell/libertas/Makefile              | 5 +++++
 drivers/net/wireless/purelifi/plfxlc/usb.c                  | 4 ++--
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c        | 5 +----
 8 files changed, 25 insertions(+), 8 deletions(-)

-- 
2.34.3

[PATCH net-next 1/8] wifi: plfxlc: remove redundant NULL-check for GCC 12

[Jakub Kicinski]

GCC is upset that we check the return value of plfxlc_usb_dev()
even tho it can't be NULL:

drivers/net/wireless/purelifi/plfxlc/usb.c: In function ‘resume’:
drivers/net/wireless/purelifi/plfxlc/usb.c:840:20: warning: the comparison will always evaluate as ‘true’ for the address of ‘dev’ will never be NULL [-Waddress]
  840 |         if (!pl || !plfxlc_usb_dev(pl))
      |                    ^

plfxlc_usb_dev() returns an address of one of the members of pl,
so it's safe to drop these checks.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: srini.raju@purelifi.com
CC: kvalo@kernel.org
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/purelifi/plfxlc/usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/purelifi/plfxlc/usb.c b/drivers/net/wireless/purelifi/plfxlc/usb.c
index d0e98b2f1365..8519cf0adfff 100644
--- a/drivers/net/wireless/purelifi/plfxlc/usb.c
+++ b/drivers/net/wireless/purelifi/plfxlc/usb.c
@@ -824,7 +824,7 @@ static int suspend(struct usb_interface *interface,
 	struct plfxlc_usb *pl = get_plfxlc_usb(interface);
 	struct plfxlc_mac *mac = plfxlc_usb_to_mac(pl);
 
-	if (!pl || !plfxlc_usb_dev(pl))
+	if (!pl)
 		return -ENODEV;
 	if (pl->initialized == 0)
 		return 0;
@@ -837,7 +837,7 @@ static int resume(struct usb_interface *interface)
 {
 	struct plfxlc_usb *pl = get_plfxlc_usb(interface);
 
-	if (!pl || !plfxlc_usb_dev(pl))
+	if (!pl)
 		return -ENODEV;
 	if (pl->was_running)
 		plfxlc_usb_resume(pl);
-- 
2.34.3

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

[Jakub Kicinski]

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
+
 ath9k_hw-$(CONFIG_ATH9K_WOW) += ar9003_wow.o
 
 ath9k_hw-$(CONFIG_ATH9K_BTCOEX_SUPPORT) += btcoex.o \
-- 
2.34.3

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

[PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12

[Jakub Kicinski]

The .value is a two-dim array, not a pointer.

struct iqk_matrix_regs {
	bool iqk_done;
        long value[1][IQK_MATRIX_REG_NUM];
};

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: pkshih@realtek.com
CC: kvalo@kernel.org
CC: keescook@chromium.org
CC: colin.king@intel.com
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
index 51fe51bb0504..15e6a6aded31 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
@@ -2386,10 +2386,7 @@ void rtl92d_phy_reload_iqk_setting(struct ieee80211_hw *hw, u8 channel)
 			rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
 				"Just Read IQK Matrix reg for channel:%d....\n",
 				channel);
-			if ((rtlphy->iqk_matrix[indexforchannel].
-			     value[0] != NULL)
-				/*&&(regea4 != 0) */)
-				_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
+			_rtl92d_phy_patha_fill_iqk_matrix(hw, true,
 					rtlphy->iqk_matrix[
 					indexforchannel].value,	0,
 					(rtlphy->iqk_matrix[
-- 
2.34.3

RE: [PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12

[Ping-Ke Shih]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Saturday, May 21, 2022 3:43 AM
> To: kvalo@kernel.org; johannes@sipsolutions.net
> Cc: netdev@vger.kernel.org; linux-wireless@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>; Pkshih
> <pkshih@realtek.com>; keescook@chromium.org; colin.king@intel.com
> Subject: [PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12
> 
> The .value is a two-dim array, not a pointer.
> 
> struct iqk_matrix_regs {
> 	bool iqk_done;
>         long value[1][IQK_MATRIX_REG_NUM];
> };
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
> CC: pkshih@realtek.com
> CC: kvalo@kernel.org
> CC: keescook@chromium.org
> CC: colin.king@intel.com
> CC: linux-wireless@vger.kernel.org
> ---
>  drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> index 51fe51bb0504..15e6a6aded31 100644
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/phy.c
> @@ -2386,10 +2386,7 @@ void rtl92d_phy_reload_iqk_setting(struct ieee80211_hw *hw, u8 channel)
>  			rtl_dbg(rtlpriv, COMP_SCAN, DBG_LOUD,
>  				"Just Read IQK Matrix reg for channel:%d....\n",
>  				channel);
> -			if ((rtlphy->iqk_matrix[indexforchannel].
> -			     value[0] != NULL)

This is a typo since initial commit. Correct it by
-			     value[0] != NULL)
+			     value[0][0] != 0)

So, NACK this patch.

--
Ping-Ke

Re: [PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12

[Jakub Kicinski]

On Mon, 23 May 2022 02:35:32 +0000 Ping-Ke Shih wrote:
> This is a typo since initial commit. Correct it by
> -			     value[0] != NULL)
> +			     value[0][0] != 0)
> 
> So, NACK this patch.

Too, late, the patches were already applied, sorry. Please post a fixup.

RE: [PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12

[Ping-Ke Shih]

> -----Original Message-----
> From: Jakub Kicinski <kuba@kernel.org>
> Sent: Monday, May 23, 2022 11:57 PM
> To: Ping-Ke Shih <pkshih@realtek.com>
> Cc: kvalo@kernel.org; johannes@sipsolutions.net; netdev@vger.kernel.org; linux-wireless@vger.kernel.org;
> keescook@chromium.org; colin.king@intel.com
> Subject: Re: [PATCH net-next 3/8] wifi: rtlwifi: remove always-true condition pointed out by GCC 12
> 
> On Mon, 23 May 2022 02:35:32 +0000 Ping-Ke Shih wrote:
> > This is a typo since initial commit. Correct it by
> > -			     value[0] != NULL)
> > +			     value[0][0] != 0)
> >
> > So, NACK this patch.
> 
> Too, late, the patches were already applied, sorry. Please post a fixup.

I have sent a patch to correct it: 
https://lore.kernel.org/linux-wireless/20220524003750.3989-1-pkshih@realtek.com/T/#u

Please take it into net-next tree.

Thank you
Ping-Ke

[PATCH net-next 4/8] wifi: ath6k: silence false positive -Wno-dangling-pointer warning on GCC 12

[Jakub Kicinski]

For some reason GCC 12 decided to complain about the common
pattern of queuing an object onto a list on the stack in ath6k:

    inlined from ‘ath6kl_htc_mbox_tx’ at drivers/net/wireless/ath/ath6kl/htc_mbox.c:1142:3:
include/linux/list.h:74:19: warning: storing the address of local variable ‘queue’ in ‘*&packet_15(D)->list.prev’ [-Wdangling-pointer=]
   74 |         new->prev = prev;
      |         ~~~~~~~~~~^~~~~~

Move the warning to W=1, hopefully it goes away with a compiler
update.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: kvalo@kernel.org
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/ath/ath6kl/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath6kl/Makefile b/drivers/net/wireless/ath/ath6kl/Makefile
index dc2b3b46781e..01cc0d50fee6 100644
--- a/drivers/net/wireless/ath/ath6kl/Makefile
+++ b/drivers/net/wireless/ath/ath6kl/Makefile
@@ -36,6 +36,11 @@ ath6kl_core-y += wmi.o
 ath6kl_core-y += core.o
 ath6kl_core-y += recovery.o
 
+# FIXME: temporarily silence -Wdangling-pointer on non W=1+ builds
+ifndef KBUILD_EXTRA_WARN
+CFLAGS_htc_mbox.o += -Wno-dangling-pointer
+endif
+
 ath6kl_core-$(CONFIG_NL80211_TESTMODE) += testmode.o
 ath6kl_core-$(CONFIG_ATH6KL_TRACING) += trace.o
 
-- 
2.34.3

[PATCH net-next 5/8] wifi: iwlwifi: use unsigned to silence a GCC 12 warning

[Jakub Kicinski]

GCC 12 says:

drivers/net/wireless/intel/iwlwifi/mvm/sta.c:1076:37: warning: array subscript -1 is below array bounds of ‘struct iwl_mvm_tid_data[9]’ [-Warray-bounds]
 1076 |                 if (mvmsta->tid_data[tid].state != IWL_AGG_OFF)
      |                     ~~~~~~~~~~~~~~~~^~~~~

Whatever, tid is a bit from for_each_set_bit(), it's clearly unsigned.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: gregory.greenman@intel.com
CC: kvalo@kernel.org
CC: luciano.coelho@intel.com
CC: johannes.berg@intel.com
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/intel/iwlwifi/mvm/sta.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
index 406f0a50a5bf..bbb1522e7280 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/sta.c
@@ -1058,7 +1058,7 @@ static bool iwl_mvm_remove_inactive_tids(struct iwl_mvm *mvm,
 					 unsigned long *unshare_queues,
 					 unsigned long *changetid_queues)
 {
-	int tid;
+	unsigned int tid;
 
 	lockdep_assert_held(&mvmsta->lock);
 	lockdep_assert_held(&mvm->mutex);
-- 
2.34.3

[PATCH net-next 6/8] wifi: brcmfmac: work around a GCC 12 -Warray-bounds warning

[Jakub Kicinski]

GCC 12 really doesn't like partial struct allocations:

drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:2202:32: warning: array subscript ‘struct brcmf_ext_join_params_le[0]’ is partly outside array bounds of ‘void[70]’ [-Warray-bounds]
 2202 |                 ext_join_params->scan_le.passive_time =
      |                                ^~

brcmfmac is trying to save 2 bytes at the end by either allocating
or not allocating a channel member. Let's keep @join_params_size
the "right" size but kmalloc() the full structure.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: aspriel@gmail.com
CC: franky.lin@broadcom.com
CC: hante.meuleman@broadcom.com
CC: kvalo@kernel.org
CC: ALSI@bang-olufsen.dk
CC: linux-wireless@vger.kernel.org
CC: brcm80211-dev-list.pdl@broadcom.com
CC: SHA-cyfmac-dev-list@infineon.com
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index 360b103fe898..605206abe424 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -2167,7 +2167,7 @@ brcmf_cfg80211_connect(struct wiphy *wiphy, struct net_device *ndev,
 		offsetof(struct brcmf_assoc_params_le, chanspec_list);
 	if (cfg->channel)
 		join_params_size += sizeof(u16);
-	ext_join_params = kzalloc(join_params_size, GFP_KERNEL);
+	ext_join_params = kzalloc(sizeof(*ext_join_params), GFP_KERNEL);
 	if (ext_join_params == NULL) {
 		err = -ENOMEM;
 		goto done;
-- 
2.34.3

[PATCH net-next 7/8] wifi: libertas: silence a GCC 12 -Warray-bounds warning

[Jakub Kicinski]

This driver does a lot of casting of smaller buffers to
a larger command response struct, GCC 12 does not like that:

drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
 1198 |                       "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
      |                                                               ^~

Annoyingly it's not clever enough to recognize which fields
are safe to access, so move this warning to W=1 for now.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: kvalo@kernel.org
CC: libertas-dev@lists.infradead.org
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/marvell/libertas/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/marvell/libertas/Makefile b/drivers/net/wireless/marvell/libertas/Makefile
index 41b9b440a542..da4ea5a0812c 100644
--- a/drivers/net/wireless/marvell/libertas/Makefile
+++ b/drivers/net/wireless/marvell/libertas/Makefile
@@ -10,6 +10,11 @@ libertas-y += tx.o
 libertas-y += firmware.o
 libertas-$(CONFIG_LIBERTAS_MESH) += mesh.o
 
+# FIXME: temporarily silence -Warray-bounds on non W=1+ builds
+ifndef KBUILD_EXTRA_WARN
+CFLAGS_cfg.o += -Wno-array-bounds
+endif
+
 usb8xxx-objs += if_usb.o
 libertas_cs-objs += if_cs.o
 libertas_sdio-objs += if_sdio.o
-- 
2.34.3

Re: [PATCH net-next 7/8] wifi: libertas: silence a GCC 12 -Warray-bounds warning

[Johannes Berg]

On Fri, 2022-05-20 at 12:43 -0700, Jakub Kicinski wrote:
> This driver does a lot of casting of smaller buffers to
> a larger command response struct, GCC 12 does not like that:
> 
> drivers/net/wireless/marvell/libertas/cfg.c:1198:63: warning: array subscript ‘struct cmd_ds_802_11_associate_response[0]’ is partly outside array bounds of ‘unsigned char[203]’ [-Warray-bounds]
>  1198 |                       "aid 0x%04x\n", status, le16_to_cpu(resp->statuscode),
>       |                                                               ^~
> 

I had a similar issue in our driver, and I could work around it there
with a simple cast ... here not, but perhaps we should consider
something like the below?

johannes

diff --git a/drivers/net/wireless/marvell/libertas/cfg.c
b/drivers/net/wireless/marvell/libertas/cfg.c
index 4e3de684928b..b0b3f59dabc6 100644
--- a/drivers/net/wireless/marvell/libertas/cfg.c
+++ b/drivers/net/wireless/marvell/libertas/cfg.c
@@ -1053,7 +1053,6 @@ static int lbs_set_authtype(struct lbs_private
*priv,
  */
 #define LBS_ASSOC_MAX_CMD_SIZE                     \
 	(sizeof(struct cmd_ds_802_11_associate)    \
-	 - 512 /* cmd_ds_802_11_associate.iebuf */ \
 	 + LBS_MAX_SSID_TLV_SIZE                   \
 	 + LBS_MAX_CHANNEL_TLV_SIZE                \
 	 + LBS_MAX_CF_PARAM_TLV_SIZE               \
@@ -1130,8 +1129,7 @@ static int lbs_associate(struct lbs_private *priv,
 	if (sme->ie && sme->ie_len)
 		pos += lbs_add_wpa_tlv(pos, sme->ie, sme->ie_len);
 
-	len = (sizeof(*cmd) - sizeof(cmd->iebuf)) +
-		(u16)(pos - (u8 *) &cmd->iebuf);
+	len = sizeof(*cmd) + (u16)(pos - (u8 *) &cmd->iebuf);
 	cmd->hdr.size = cpu_to_le16(len);
 
 	lbs_deb_hex(LBS_DEB_ASSOC, "ASSOC_CMD", (u8 *) cmd,
diff --git a/drivers/net/wireless/marvell/libertas/host.h
b/drivers/net/wireless/marvell/libertas/host.h
index ceff4b92e7a1..a202b716ad5d 100644
--- a/drivers/net/wireless/marvell/libertas/host.h
+++ b/drivers/net/wireless/marvell/libertas/host.h
@@ -528,7 +528,8 @@ struct cmd_ds_802_11_associate {
 	__le16 listeninterval;
 	__le16 bcnperiod;
 	u8 dtimperiod;
-	u8 iebuf[512];    /* Enough for required and most optional IEs
*/
+	/* 512 permitted - enough for required and most optional IEs */
+	u8 iebuf[];
 } __packed;
 
 struct cmd_ds_802_11_associate_response {
@@ -537,7 +538,8 @@ struct cmd_ds_802_11_associate_response {
 	__le16 capability;
 	__le16 statuscode;
 	__le16 aid;
-	u8 iebuf[512];
+	/* max 512 */
+	u8 iebuf[];
 } __packed;
 
 struct cmd_ds_802_11_set_wep {

Re: [PATCH net-next 7/8] wifi: libertas: silence a GCC 12 -Warray-bounds warning

[Jakub Kicinski]

On Sun, 22 May 2022 00:03:22 +0200 Johannes Berg wrote:
> I had a similar issue in our driver, and I could work around it there
> with a simple cast ... here not, but perhaps we should consider
> something like the below?

Excellent, LGTM. Would you be willing to submit this officially?
I'll drop patch 7 for now.

[PATCH net-next 8/8] wifi: carl9170: silence a GCC 12 -Warray-bounds warning

[Jakub Kicinski]

carl9170 has a big union (struct carl9170_cmd) with all the command
types in it. But it allocates buffers only large enough for a given
command. This upsets GCC 12:

drivers/net/wireless/ath/carl9170/cmd.c:125:30: warning: array subscript ‘struct carl9170_cmd[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
  125 |                 tmp->hdr.cmd = cmd;
      |                 ~~~~~~~~~~~~~^~~~~

Punt the warning to W=1 for now. Hopefully GCC will learn to
recognize which fields are in-bounds.

Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
CC: chunkeey@googlemail.com
CC: kvalo@kernel.org
CC: linux-wireless@vger.kernel.org
---
 drivers/net/wireless/ath/carl9170/Makefile | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/carl9170/Makefile b/drivers/net/wireless/ath/carl9170/Makefile
index 1a81868ce26d..7463baa62fa8 100644
--- a/drivers/net/wireless/ath/carl9170/Makefile
+++ b/drivers/net/wireless/ath/carl9170/Makefile
@@ -3,3 +3,8 @@ carl9170-objs := main.o usb.o cmd.o mac.o phy.o led.o fw.o tx.o rx.o
 carl9170-$(CONFIG_CARL9170_DEBUGFS) += debug.o
 
 obj-$(CONFIG_CARL9170) += carl9170.o
+
+# FIXME: temporarily silence -Warray-bounds on non W=1+ builds
+ifndef KBUILD_EXTRA_WARN
+CFLAGS_cmd.o += -Wno-array-bounds
+endif
-- 
2.34.3

Re: [PATCH net-next 8/8] wifi: carl9170: silence a GCC 12 -Warray-bounds warning

[Christian Lamparter]

On 20/05/2022 21:43, Jakub Kicinski wrote:
> carl9170 has a big union (struct carl9170_cmd) with all the command
> types in it. But it allocates buffers only large enough for a given
> command. This upsets GCC 12:
> 
> drivers/net/wireless/ath/carl9170/cmd.c:125:30: warning: array subscript ‘struct carl9170_cmd[0]’ is partly outside array bounds of ‘unsigned char[8]’ [-Warray-bounds]
>    125 |                 tmp->hdr.cmd = cmd;
>        |                 ~~~~~~~~~~~~~^~~~~
> 
> Punt the warning to W=1 for now. Hopefully GCC will learn to
> recognize which fields are in-bounds.
> 
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Acked-by: Christian Lamparter <chunkeey@gmail.com>

Re: [PATCH net-next 0/8] Fix/silence GCC 12 warnings in drivers/net/wireless/

[Kalle Valo]

Jakub Kicinski <kuba@kernel.org> writes:

> Hi Kalle & Johannes,
>
> as mentioned off list we'd like to get GCC 12 warnings quashed.
> This set takes care of the warnings we have in drivers/net/wireless/
> mostly by relegating them to W=1/W=2 builds.
>
> Is it okay for us to take this directly to net-next?
> Or perhaps via wireless-next with a quick PR by Monday?

We are not planning to submit any new pull requests so please take it
directly net-next.

> Jakub Kicinski (8):
>   wifi: plfxlc: remove redundant NULL-check for GCC 12
>   wifi: ath9k: silence array-bounds warning on GCC 12
>   wifi: rtlwifi: remove always-true condition pointed out by GCC 12
>   wifi: ath6k: silence false positive -Wno-dangling-pointer warning on
>     GCC 12
>   wifi: iwlwifi: use unsigned to silence a GCC 12 warning
>   wifi: brcmfmac: work around a GCC 12 -Warray-bounds warning
>   wifi: libertas: silence a GCC 12 -Warray-bounds warning
>   wifi: carl9170: silence a GCC 12 -Warray-bounds warning

Like I mentioned in the other email I don't really like these but I
understood they are urgent so:

Acked-by: Kalle Valo <kvalo@kernel.org>

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

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