[PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays

public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays
       [not found] <20230913065421.12615-1-juerg.haefliger@canonical.com>
@ 2023-09-14  7:02 ` Juerg Haefliger
  2023-09-14 16:52   ` Kees Cook
                     ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Juerg Haefliger @ 2023-09-14  7:02 UTC (permalink / raw)
  To: juerg.haefliger
  Cc: SHA-cyfmac-dev-list, aspriel, brcm80211-dev-list.pdl, franky.lin,
	gustavoars, hante.meuleman, hdegoede, keescook, kvalo,
	linus.walleij, linux-kernel, linux-wireless, marcan, ryohei.kondo,
	stable

Since commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC"),
UBSAN_BOUNDS no longer pretends 1-element arrays are unbounded. Walking
'element' and 'channel_list' will trigger warnings, so make them proper
flexible arrays.

False positive warnings were:

  UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6984:20
  index 1 is out of range for type '__le32 [1]'

  UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1126:27
  index 1 is out of range for type '__le16 [1]'

for these lines of code:

  6884  ch.chspec = (u16)le32_to_cpu(list->element[i]);

  1126  params_le->channel_list[i] = cpu_to_le16(chanspec);

Cc: stable@vger.kernel.org # 6.5+
Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>

---
v2:
  - Use element[] instead of DFA() in brcmf_chanspec_list.
  - Add Cc: stable tag
---
 .../wireless/broadcom/brcm80211/brcmfmac/fwil_types.h    | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
index bece26741d3a..611d1a6aabb9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
@@ -442,7 +442,12 @@ struct brcmf_scan_params_v2_le {
 				 * fixed parameter portion is assumed, otherwise
 				 * ssid in the fixed portion is ignored
 				 */
-	__le16 channel_list[1];	/* list of chanspecs */
+	union {
+		__le16 padding;	/* Reserve space for at least 1 entry for abort
+				 * which uses an on stack brcmf_scan_params_v2_le
+				 */
+		DECLARE_FLEX_ARRAY(__le16, channel_list);	/* chanspecs */
+	};
 };
 
 struct brcmf_scan_results {
@@ -702,7 +707,7 @@ struct brcmf_sta_info_le {
 
 struct brcmf_chanspec_list {
 	__le32	count;		/* # of entries */
-	__le32	element[1];	/* variable length uint32 list */
+	__le32  element[];	/* variable length uint32 list */
 };
 
 /*
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays
  2023-09-14  7:02 ` [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays Juerg Haefliger
@ 2023-09-14 16:52   ` Kees Cook
  2023-09-14 17:19   ` Gustavo A. R. Silva
  2023-09-18 13:20   ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2023-09-14 16:52 UTC (permalink / raw)
  To: Juerg Haefliger
  Cc: SHA-cyfmac-dev-list, aspriel, brcm80211-dev-list.pdl, franky.lin,
	gustavoars, hante.meuleman, hdegoede, kvalo, linus.walleij,
	linux-kernel, linux-wireless, marcan, ryohei.kondo, stable

On Thu, Sep 14, 2023 at 09:02:27AM +0200, Juerg Haefliger wrote:
> Since commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC"),
> UBSAN_BOUNDS no longer pretends 1-element arrays are unbounded. Walking
> 'element' and 'channel_list' will trigger warnings, so make them proper
> flexible arrays.
> 
> False positive warnings were:
> 
>   UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6984:20
>   index 1 is out of range for type '__le32 [1]'
> 
>   UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1126:27
>   index 1 is out of range for type '__le16 [1]'
> 
> for these lines of code:
> 
>   6884  ch.chspec = (u16)le32_to_cpu(list->element[i]);
> 
>   1126  params_le->channel_list[i] = cpu_to_le16(chanspec);
> 
> Cc: stable@vger.kernel.org # 6.5+
> Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays
  2023-09-14  7:02 ` [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays Juerg Haefliger
  2023-09-14 16:52   ` Kees Cook
@ 2023-09-14 17:19   ` Gustavo A. R. Silva
  2023-09-18 13:20   ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Gustavo A. R. Silva @ 2023-09-14 17:19 UTC (permalink / raw)
  To: Juerg Haefliger
  Cc: SHA-cyfmac-dev-list, aspriel, brcm80211-dev-list.pdl, franky.lin,
	gustavoars, hante.meuleman, hdegoede, keescook, kvalo,
	linus.walleij, linux-kernel, linux-wireless, marcan, ryohei.kondo,
	stable, linux-hardening



On 9/14/23 01:02, Juerg Haefliger wrote:
> Since commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC"),
> UBSAN_BOUNDS no longer pretends 1-element arrays are unbounded. Walking
> 'element' and 'channel_list' will trigger warnings, so make them proper
> flexible arrays.
> 
> False positive warnings were:
> 
>    UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6984:20
>    index 1 is out of range for type '__le32 [1]'
> 
>    UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1126:27
>    index 1 is out of range for type '__le16 [1]'
> 
> for these lines of code:
> 
>    6884  ch.chspec = (u16)le32_to_cpu(list->element[i]);
> 
>    1126  params_le->channel_list[i] = cpu_to_le16(chanspec);
> 
> Cc: stable@vger.kernel.org # 6.5+
> Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> 
> ---
> v2:
>    - Use element[] instead of DFA() in brcmf_chanspec_list.
>    - Add Cc: stable tag
> ---
>   .../wireless/broadcom/brcm80211/brcmfmac/fwil_types.h    | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> index bece26741d3a..611d1a6aabb9 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fwil_types.h
> @@ -442,7 +442,12 @@ struct brcmf_scan_params_v2_le {
>   				 * fixed parameter portion is assumed, otherwise
>   				 * ssid in the fixed portion is ignored
>   				 */
> -	__le16 channel_list[1];	/* list of chanspecs */
> +	union {
> +		__le16 padding;	/* Reserve space for at least 1 entry for abort
> +				 * which uses an on stack brcmf_scan_params_v2_le
> +				 */
> +		DECLARE_FLEX_ARRAY(__le16, channel_list);	/* chanspecs */
> +	};
>   };
>   
>   struct brcmf_scan_results {
> @@ -702,7 +707,7 @@ struct brcmf_sta_info_le {
>   
>   struct brcmf_chanspec_list {
>   	__le32	count;		/* # of entries */
> -	__le32	element[1];	/* variable length uint32 list */
> +	__le32  element[];	/* variable length uint32 list */
>   };
>   
>   /*

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays
  2023-09-14  7:02 ` [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays Juerg Haefliger
  2023-09-14 16:52   ` Kees Cook
  2023-09-14 17:19   ` Gustavo A. R. Silva
@ 2023-09-18 13:20   ` Kalle Valo
  2 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2023-09-18 13:20 UTC (permalink / raw)
  To: Juerg Haefliger
  Cc: juerg.haefliger, SHA-cyfmac-dev-list, aspriel,
	brcm80211-dev-list.pdl, franky.lin, gustavoars, hante.meuleman,
	hdegoede, keescook, linus.walleij, linux-kernel, linux-wireless,
	marcan, ryohei.kondo, stable

Juerg Haefliger <juerg.haefliger@canonical.com> wrote:

> Since commit 2d47c6956ab3 ("ubsan: Tighten UBSAN_BOUNDS on GCC"),
> UBSAN_BOUNDS no longer pretends 1-element arrays are unbounded. Walking
> 'element' and 'channel_list' will trigger warnings, so make them proper
> flexible arrays.
> 
> False positive warnings were:
> 
>   UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:6984:20
>   index 1 is out of range for type '__le32 [1]'
> 
>   UBSAN: array-index-out-of-bounds in drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:1126:27
>   index 1 is out of range for type '__le16 [1]'
> 
> for these lines of code:
> 
>   6884  ch.chspec = (u16)le32_to_cpu(list->element[i]);
> 
>   1126  params_le->channel_list[i] = cpu_to_le16(chanspec);
> 
> Cc: stable@vger.kernel.org # 6.5+
> Signed-off-by: Juerg Haefliger <juerg.haefliger@canonical.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Patch applied to wireless.git, thanks.

4fed494abcd4 wifi: brcmfmac: Replace 1-element arrays with flexible arrays

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230914070227.12028-1-juerg.haefliger@canonical.com/

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-09-18 17:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20230913065421.12615-1-juerg.haefliger@canonical.com>
2023-09-14  7:02 ` [PATCH v2] wifi: brcmfmac: Replace 1-element arrays with flexible arrays Juerg Haefliger
2023-09-14 16:52   ` Kees Cook
2023-09-14 17:19   ` Gustavo A. R. Silva
2023-09-18 13:20   ` Kalle Valo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox