Linux wireless drivers development
 help / color / mirror / Atom feed
* [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
@ 2026-07-09 10:08 Doruk Tan Ozturk
  2026-07-15 14:40 ` Francesco Dolcini
  2026-07-15 18:55 ` [PATCH v2] " Doruk Tan Ozturk
  0 siblings, 2 replies; 6+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-09 10:08 UTC (permalink / raw)
  To: Brian Norris
  Cc: Francesco Dolcini, Kees Cook, linux-wireless, linux-kernel,
	stable, Doruk Tan Ozturk

mwifiex_update_bss_desc_with_ie() records raw pointers to the HT
Capabilities, HT Operation, VHT Capabilities, VHT Operation, 20/40 BSS
Coexistence and Operating Mode Notification elements taken straight out
of a beacon/probe-response buffer, without checking that each element is
long enough for the fixed-size structure that later consumers read. The
buffer is a tight kmemdup() of the on-air IEs (beacon_buf_size ==
ies->len), so a truncated element placed last leaves the stored pointer
one past the end of the allocation.

At association time these pointers are dereferenced at fixed offsets
regardless of the on-air length: mwifiex_cmd_append_11n_tlv() memcpy()s
sizeof(struct ieee80211_ht_cap) (26 bytes) from bcn_ht_cap and reads
bcn_ht_oper->ht_param, and mwifiex_cmd_append_11ac_tlv() memcpy()s
sizeof(struct ieee80211_vht_cap) (12 bytes) from bcn_vht_cap and reads
bcn_vht_oper->chan_width. A nearby AP (rogue / evil-twin; an open SSID
needs no credentials) advertising a BSS with a truncated HT/VHT cap
element therefore triggers a slab out-of-bounds read on the victim's
association attempt. This out-of-bounds read is the primary issue.

For the HT-Cap copy the over-read bytes are additionally placed into the
outgoing association request, so a limited amount of adjacent heap memory
can leak over the air. In station mode this is small (single-digit
bytes), because mwifiex_fill_cap_info() rewrites most of the copied
HT-Cap before transmission; the leak is a secondary effect.

mwifiex_set_sta_ht_cap() has the same missing-length pattern: in uAP mode
it reads two bytes of ieee80211_ht_cap.cap_info from a
cfg80211_find_ie(WLAN_EID_HT_CAPABILITY) result in a client association
request without checking the element length, a 1-2 byte out-of-bounds
read (used only to select an A-MSDU size, not leaked).

Reject (skip) any of these elements whose payload is shorter than the
structure the driver later reads, matching the length validation the
FH/DS/CF/IBSS parameter-set cases in the same beacon parser already
perform.

No dynamic reproducer: mwifiex is a fullmac driver for Marvell hardware
with no mac80211_hwsim equivalent, so this was confirmed by source and
structure-offset analysis only.

Found by 0sec automated security-research tooling (https://0sec.ai).

Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
 drivers/net/wireless/marvell/mwifiex/scan.c | 12 ++++++++++++
 drivers/net/wireless/marvell/mwifiex/util.c |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 97c0ec3b822e..997e7e19525b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1384,6 +1384,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 							bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_ht_cap))
+				break;
 			bss_entry->bcn_ht_cap = (struct ieee80211_ht_cap *)
 					(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1392,6 +1394,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_ht_operation))
+				break;
 			bss_entry->bcn_ht_oper =
 				(struct ieee80211_ht_operation *)(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1400,6 +1404,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_vht_cap))
+				break;
 			bss_entry->disable_11ac = false;
 			bss_entry->bcn_vht_cap =
 				(void *)(current_ptr +
@@ -1409,6 +1415,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					      bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_vht_operation))
+				break;
 			bss_entry->bcn_vht_oper =
 				(void *)(current_ptr +
 					 sizeof(struct ieee_types_header));
@@ -1417,6 +1425,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					      bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_BSS_COEX_2040:
+			if (!element_len)
+				break;
 			bss_entry->bcn_bss_co_2040 = current_ptr;
 			bss_entry->bss_co_2040_offset =
 				(u16) (current_ptr - bss_entry->beacon_buf);
@@ -1427,6 +1437,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 				(u16) (current_ptr - bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_OPMODE_NOTIF:
+			if (!element_len)
+				break;
 			bss_entry->oper_mode = (void *)current_ptr;
 			bss_entry->oper_mode_offset =
 					(u16)((u8 *)bss_entry->oper_mode -
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 7d3631d21223..844223c04e2e 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -721,7 +721,7 @@ mwifiex_set_sta_ht_cap(struct mwifiex_private *priv, const u8 *ies,
 
 	ht_cap_ie = (void *)cfg80211_find_ie(WLAN_EID_HT_CAPABILITY, ies,
 					     ies_len);
-	if (ht_cap_ie) {
+	if (ht_cap_ie && ht_cap_ie->len >= sizeof(struct ieee80211_ht_cap)) {
 		ht_cap = (void *)(ht_cap_ie + 1);
 		node->is_11n_enabled = 1;
 		node->max_amsdu = le16_to_cpu(ht_cap->cap_info) &
-- 
2.43.0


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

* Re: [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
  2026-07-09 10:08 [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths Doruk Tan Ozturk
@ 2026-07-15 14:40 ` Francesco Dolcini
  2026-07-15 16:28   ` Doruk Tan Ozturk
  2026-07-15 18:55 ` [PATCH v2] " Doruk Tan Ozturk
  1 sibling, 1 reply; 6+ messages in thread
From: Francesco Dolcini @ 2026-07-15 14:40 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: Brian Norris, Francesco Dolcini, Kees Cook, linux-wireless,
	linux-kernel, stable

On Thu, Jul 09, 2026 at 12:08:00PM +0200, Doruk Tan Ozturk wrote:
> mwifiex_update_bss_desc_with_ie() records raw pointers to the HT
> Capabilities, HT Operation, VHT Capabilities, VHT Operation, 20/40 BSS
> Coexistence and Operating Mode Notification elements taken straight out
> of a beacon/probe-response buffer, without checking that each element is
> long enough for the fixed-size structure that later consumers read. The
> buffer is a tight kmemdup() of the on-air IEs (beacon_buf_size ==
> ies->len), so a truncated element placed last leaves the stored pointer
> one past the end of the allocation.
> 
> At association time these pointers are dereferenced at fixed offsets
> regardless of the on-air length: mwifiex_cmd_append_11n_tlv() memcpy()s
> sizeof(struct ieee80211_ht_cap) (26 bytes) from bcn_ht_cap and reads
> bcn_ht_oper->ht_param, and mwifiex_cmd_append_11ac_tlv() memcpy()s
> sizeof(struct ieee80211_vht_cap) (12 bytes) from bcn_vht_cap and reads
> bcn_vht_oper->chan_width. A nearby AP (rogue / evil-twin; an open SSID
> needs no credentials) advertising a BSS with a truncated HT/VHT cap
> element therefore triggers a slab out-of-bounds read on the victim's
> association attempt. This out-of-bounds read is the primary issue.
> 
> For the HT-Cap copy the over-read bytes are additionally placed into the
> outgoing association request, so a limited amount of adjacent heap memory
> can leak over the air. In station mode this is small (single-digit
> bytes), because mwifiex_fill_cap_info() rewrites most of the copied
> HT-Cap before transmission; the leak is a secondary effect.
> 
> mwifiex_set_sta_ht_cap() has the same missing-length pattern: in uAP mode
> it reads two bytes of ieee80211_ht_cap.cap_info from a
> cfg80211_find_ie(WLAN_EID_HT_CAPABILITY) result in a client association
> request without checking the element length, a 1-2 byte out-of-bounds
> read (used only to select an A-MSDU size, not leaked).
> 
> Reject (skip) any of these elements whose payload is shorter than the
> structure the driver later reads, matching the length validation the
> FH/DS/CF/IBSS parameter-set cases in the same beacon parser already
> perform.
> 
> No dynamic reproducer: mwifiex is a fullmac driver for Marvell hardware
> with no mac80211_hwsim equivalent, so this was confirmed by source and
> structure-offset analysis only.

Were you able to test that this is not breaking the driver functionality?

> 
> Found by 0sec automated security-research tooling (https://0sec.ai).
> 
> Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
> Cc: stable@vger.kernel.org
> Assisted-by: 0sec:claude-opus-4-8
> Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
> ---
>  drivers/net/wireless/marvell/mwifiex/scan.c | 12 ++++++++++++
>  drivers/net/wireless/marvell/mwifiex/util.c |  2 +-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
> index 97c0ec3b822e..997e7e19525b 100644
> --- a/drivers/net/wireless/marvell/mwifiex/scan.c
> +++ b/drivers/net/wireless/marvell/mwifiex/scan.c
> @@ -1384,6 +1384,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
>  							bss_entry->beacon_buf);
>  			break;
>  		case WLAN_EID_HT_CAPABILITY:
> +			if (element_len < sizeof(struct ieee80211_ht_cap))
> +				break;

why break and not return -EINVAL? (this in general, not only on this
specific one).



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

* Re: [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
  2026-07-15 14:40 ` Francesco Dolcini
@ 2026-07-15 16:28   ` Doruk Tan Ozturk
  2026-07-15 16:34     ` Francesco Dolcini
  0 siblings, 1 reply; 6+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-15 16:28 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Brian Norris, Kees Cook, linux-wireless, linux-kernel, stable

> Were you able to test that this is not breaking the driver functionality?

Unfortunately I don't own one of these cards, and mwifiex is fullmac so
there's no hwsim either, so it was source and compile only. The added checks
only skip elements too short for what the driver later reads, so they
shouldn't affect a well-formed beacon.

> why break and not return -EINVAL? (this in general, not only on this
> specific one).

To skip the one malformed optional element and keep parsing the rest of the
beacon, like the FH/DS/CF/IBSS cases just above in the same loop. Happy to
switch to -EINVAL if you'd rather reject the whole BSS.

-Doruk

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

* Re: [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
  2026-07-15 16:28   ` Doruk Tan Ozturk
@ 2026-07-15 16:34     ` Francesco Dolcini
  2026-07-15 18:33       ` Doruk Tan Ozturk
  0 siblings, 1 reply; 6+ messages in thread
From: Francesco Dolcini @ 2026-07-15 16:34 UTC (permalink / raw)
  To: Doruk Tan Ozturk
  Cc: Francesco Dolcini, Brian Norris, Kees Cook, linux-wireless,
	linux-kernel, stable

On Wed, Jul 15, 2026 at 06:28:00PM +0200, Doruk Tan Ozturk wrote:
> > why break and not return -EINVAL? (this in general, not only on this
> > specific one).
> 
> To skip the one malformed optional element and keep parsing the rest of the
> beacon, like the FH/DS/CF/IBSS cases just above in the same loop. Happy to
> switch to -EINVAL if you'd rather reject the whole BSS.

maybe I am missing something, but I see return -EINVAL; when the packet
is malformed. see for example

		case WLAN_EID_VENDOR_SPECIFIC:
			vendor_ie = (struct ieee_types_vendor_specific *)
					current_ptr;

			/* 802.11 requires at least 3-byte OUI. */
			if (element_len < sizeof(vendor_ie->vend_hdr.oui.oui))
				return -EINVAL;


My advise would be to do the same we are already doing, and unless I am
wrong, it means that we skip the whole frame if a corruption is
detected.

Francesco



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

* Re: [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
  2026-07-15 16:34     ` Francesco Dolcini
@ 2026-07-15 18:33       ` Doruk Tan Ozturk
  0 siblings, 0 replies; 6+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-15 18:33 UTC (permalink / raw)
  To: Francesco Dolcini
  Cc: Brian Norris, Kees Cook, linux-wireless, linux-kernel, stable

> My advise would be to do the same we are already doing, and unless I am
> wrong, it means that we skip the whole frame if a corruption is detected.

Makes sense, I'll switch these to -EINVAL and send a v2. Thanks.

-Doruk

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

* [PATCH v2] wifi: mwifiex: validate HT/VHT capability and operation IE lengths
  2026-07-09 10:08 [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths Doruk Tan Ozturk
  2026-07-15 14:40 ` Francesco Dolcini
@ 2026-07-15 18:55 ` Doruk Tan Ozturk
  1 sibling, 0 replies; 6+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-15 18:55 UTC (permalink / raw)
  To: briannorris; +Cc: francesco, kees, linux-wireless, linux-kernel, stable

mwifiex_update_bss_desc_with_ie() records raw pointers to the HT
Capabilities, HT Operation, VHT Capabilities, VHT Operation, 20/40 BSS
Coexistence and Operating Mode Notification elements taken straight out
of a beacon/probe-response buffer, without checking that each element is
long enough for the fixed-size structure that later consumers read. The
buffer is a tight kmemdup() of the on-air IEs (beacon_buf_size ==
ies->len), so a truncated element placed last leaves the stored pointer
one past the end of the allocation.

At association time these pointers are dereferenced at fixed offsets
regardless of the on-air length: mwifiex_cmd_append_11n_tlv() memcpy()s
sizeof(struct ieee80211_ht_cap) (26 bytes) from bcn_ht_cap and reads
bcn_ht_oper->ht_param, and mwifiex_cmd_append_11ac_tlv() memcpy()s
sizeof(struct ieee80211_vht_cap) (12 bytes) from bcn_vht_cap and reads
bcn_vht_oper->chan_width. A nearby AP (rogue / evil-twin; an open SSID
needs no credentials) advertising a BSS with a truncated HT/VHT cap
element therefore triggers a slab out-of-bounds read on the victim's
association attempt. This out-of-bounds read is the primary issue.

For the HT-Cap copy the over-read bytes are additionally placed into the
outgoing association request, so a limited amount of adjacent heap memory
can leak over the air. In station mode this is small (single-digit
bytes), because mwifiex_fill_cap_info() rewrites most of the copied
HT-Cap before transmission; the leak is a secondary effect.

mwifiex_set_sta_ht_cap() has the same missing-length pattern: in uAP mode
it reads two bytes of ieee80211_ht_cap.cap_info from a
cfg80211_find_ie(WLAN_EID_HT_CAPABILITY) result in a client association
request without checking the element length, a 1-2 byte out-of-bounds
read (used only to select an A-MSDU size, not leaked).

Reject the frame with -EINVAL when any of these elements is shorter than
the structure the driver later reads, matching the length validation the
FH/DS/CF/IBSS parameter-set cases in the same beacon parser already
perform. mwifiex_set_sta_ht_cap() returns void, so there the too-short
element is skipped instead.

Found by 0sec automated security-research tooling (https://0sec.ai).

Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
Cc: stable@vger.kernel.org
Assisted-by: 0sec:multi-model
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---

Changes in v2 (per Francesco Dolcini review):
 - return -EINVAL on a too-short element instead of break, matching the
   FH/DS/CF/IBSS and VENDOR_SPECIFIC cases in the same function.
   mwifiex_set_sta_ht_cap() returns void, so there it stays a skip.
 - switch the Assisted-by trailer to 0sec:multi-model.
 drivers/net/wireless/marvell/mwifiex/scan.c | 12 ++++++++++++
 drivers/net/wireless/marvell/mwifiex/util.c |  2 +-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index 97c0ec3b822e..22031faba057 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1384,6 +1384,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 							bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_ht_cap))
+				return -EINVAL;
 			bss_entry->bcn_ht_cap = (struct ieee80211_ht_cap *)
 					(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1392,6 +1394,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_HT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_ht_operation))
+				return -EINVAL;
 			bss_entry->bcn_ht_oper =
 				(struct ieee80211_ht_operation *)(current_ptr +
 					sizeof(struct ieee_types_header));
@@ -1400,6 +1404,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_CAPABILITY:
+			if (element_len < sizeof(struct ieee80211_vht_cap))
+				return -EINVAL;
 			bss_entry->disable_11ac = false;
 			bss_entry->bcn_vht_cap =
 				(void *)(current_ptr +
@@ -1409,6 +1415,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					      bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_VHT_OPERATION:
+			if (element_len < sizeof(struct ieee80211_vht_operation))
+				return -EINVAL;
 			bss_entry->bcn_vht_oper =
 				(void *)(current_ptr +
 					 sizeof(struct ieee_types_header));
@@ -1417,6 +1425,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 					      bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_BSS_COEX_2040:
+			if (!element_len)
+				return -EINVAL;
 			bss_entry->bcn_bss_co_2040 = current_ptr;
 			bss_entry->bss_co_2040_offset =
 				(u16) (current_ptr - bss_entry->beacon_buf);
@@ -1427,6 +1437,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
 				(u16) (current_ptr - bss_entry->beacon_buf);
 			break;
 		case WLAN_EID_OPMODE_NOTIF:
+			if (!element_len)
+				return -EINVAL;
 			bss_entry->oper_mode = (void *)current_ptr;
 			bss_entry->oper_mode_offset =
 					(u16)((u8 *)bss_entry->oper_mode -
diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
index 7d3631d21223..844223c04e2e 100644
--- a/drivers/net/wireless/marvell/mwifiex/util.c
+++ b/drivers/net/wireless/marvell/mwifiex/util.c
@@ -721,7 +721,7 @@ mwifiex_set_sta_ht_cap(struct mwifiex_private *priv, const u8 *ies,
 
 	ht_cap_ie = (void *)cfg80211_find_ie(WLAN_EID_HT_CAPABILITY, ies,
 					     ies_len);
-	if (ht_cap_ie) {
+	if (ht_cap_ie && ht_cap_ie->len >= sizeof(struct ieee80211_ht_cap)) {
 		ht_cap = (void *)(ht_cap_ie + 1);
 		node->is_11n_enabled = 1;
 		node->max_amsdu = le16_to_cpu(ht_cap->cap_info) &
-- 
2.43.0


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

end of thread, other threads:[~2026-07-15 18:55 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 10:08 [PATCH] wifi: mwifiex: validate HT/VHT capability and operation IE lengths Doruk Tan Ozturk
2026-07-15 14:40 ` Francesco Dolcini
2026-07-15 16:28   ` Doruk Tan Ozturk
2026-07-15 16:34     ` Francesco Dolcini
2026-07-15 18:33       ` Doruk Tan Ozturk
2026-07-15 18:55 ` [PATCH v2] " Doruk Tan Ozturk

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