[PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[HE WEI (ギカク)]

mwifiex_process_uap_event() handles EVENT_UAP_STA_ASSOC by exposing the
(re)association request IEs that the firmware copies into the event:

	sinfo->assoc_req_ies = &event->data[len];
	len = (u8 *)sinfo->assoc_req_ies - (u8 *)&event->frame_control;
	sinfo->assoc_req_ies_len = le16_to_cpu(event->len) - (u16)len;

event->len is supplied by the device firmware and is never validated,
and the subtraction is unchecked.  assoc_req_ies points into
adapter->event_body[MAX_EVENT_SIZE], a fixed-size array embedded in the
kmalloc()'d struct mwifiex_adapter.

On the ap_11n_enabled path mwifiex_set_sta_ht_cap() walks these IEs with
cfg80211_find_ie(), whose for_each_element() loop dereferences each
element header.  A firmware-reported event->len larger than the bytes
actually received makes assoc_req_ies_len describe IEs that extend past
event_body, so the walk reads out of the adapter slab object -- a
slab-out-of-bounds read (KASAN: slab-out-of-bounds in cfg80211_find_ie).
An event->len smaller than the header instead makes the int subtraction
negative, which wraps to a huge size_t when stored in assoc_req_ies_len.
The same length is handed to cfg80211_new_sta(), so a more modest
over-claim can also copy stale event_body bytes into the
NL80211_CMD_NEW_STATION notification.

A malicious or malfunctioning mwifiex device (USB/SDIO/PCIe) can deliver
such an event while the interface is in AP/uAP mode.

Validate event->len before use: reject a length that underflows the
header or that would place the IEs outside the event_body[] buffer the
event was copied into.  The bound is against event_body[MAX_EVENT_SIZE]
rather than the actually-received length because the transports store the
event differently (USB and SDIO leave the 4-byte event header in
event_skb, PCIe strips it via skb_pull), whereas event_body is the single
fixed buffer all of them copy the event into.  This is the event-path
analogue of the receive-path bounds checks added in commit 119585281617
("wifi: mwifiex: Fix OOB and integer underflow when rx packets").

Fixes: e568634ae7ac ("mwifiex: add AP event handling framework")
Signed-off-by: HE WEI (ギカク) <skyexpoc@gmail.com>
---
 .../net/wireless/marvell/mwifiex/uap_event.c   | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/uap_event.c b/drivers/net/wireless/marvell/mwifiex/uap_event.c
index 679fdae0f001..adca7da29f0f 100644
--- a/drivers/net/wireless/marvell/mwifiex/uap_event.c
+++ b/drivers/net/wireless/marvell/mwifiex/uap_event.c
@@ -126,6 +126,24 @@ int mwifiex_process_uap_event(struct mwifiex_private *priv)
 				sinfo->assoc_req_ies = &event->data[len];
 				len = (u8 *)sinfo->assoc_req_ies -
 				      (u8 *)&event->frame_control;
+
+				/*
+				 * event->len is reported by the device firmware and is not
+				 * otherwise validated.  Reject a length that underflows the
+				 * header, or that would place the association request IEs
+				 * outside the fixed-size event_body[] buffer the event was
+				 * copied into; otherwise the IE walk in
+				 * mwifiex_set_sta_ht_cap() reads past event_body and out
+				 * of the adapter slab object.
+				 */
+				if (le16_to_cpu(event->len) < len ||
+				    (u8 *)&event->frame_control + le16_to_cpu(event->len) >
+				    adapter->event_body + MAX_EVENT_SIZE) {
+					mwifiex_dbg(adapter, ERROR,
+						    "invalid STA assoc event length\n");
+					kfree(sinfo);
+					return -1;
+				}
 				sinfo->assoc_req_ies_len =
 					le16_to_cpu(event->len) - (u16)len;
 			}
-- 
2.54.0

Re: [PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[Francesco Dolcini]

Hello Brian, Johannes

On Mon, Jun 29, 2026 at 09:03:33PM +0900, HE WEI (ギカク) wrote:
> mwifiex_process_uap_event() handles EVENT_UAP_STA_ASSOC by exposing the
> (re)association request IEs that the firmware copies into the event:
> 
> 	sinfo->assoc_req_ies = &event->data[len];
> 	len = (u8 *)sinfo->assoc_req_ies - (u8 *)&event->frame_control;
> 	sinfo->assoc_req_ies_len = le16_to_cpu(event->len) - (u16)len;
> 
> event->len is supplied by the device firmware and is never validated,

I think we received a few patches that are validating the data received
from the firmware (including this one).

I did not review any of them yet, what is your opinion on those?

Should we consider the firmware trust-worth or should we validate
everything we receive from it? Is there some agreement on this topic in
general?

Francesco

Re: [PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[Johannes Berg]

On Fri, 2026-07-10 at 09:21 +0200, Francesco Dolcini wrote:
> I think we received a few patches that are validating the data received
> from the firmware (including this one).
> 
> I did not review any of them yet, what is your opinion on those?
> 
> Should we consider the firmware trust-worth or should we validate
> everything we receive from it? Is there some agreement on this topic in
> general?

I don't really know fully "in general", but I/we generally treat these
as issues. I'm not going to worry too much about them, but I think we
should fix them.

johannes

Re: [PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[skyexpoc]

[-- Attachment #1.1: Type: text/plain, Size: 1710 bytes --]

> Should we consider the firmware trust-worthy or should we validate
> everything we receive from it? Is there some agreement on this topic
> in general?

For mwifiex there's a concrete reason not to trust it: besides the
SDIO/PCIe parts, mwifiex also drives USB dongles (8797/8801/8997), so the
"firmware" can sit on a hot-pluggable, potentially attacker-supplied
device. A malformed event from such a device is the same threat model as
any other malicious USB peripheral, not just a hypothetical firmware bug.

There is also precedent inside mwifiex itself: commit 119585281617
("wifi: mwifiex: Fix OOB and integer underflow when rx packets") already
added exactly these bounds checks to the receive-data path. This patch is
the same class of fix for the uAP event path, which that commit did not
cover.

So I'd say it's consistent with how the driver already treats
firmware-reported lengths and agreed, worth fixing.

Thanks,
HE WEI（ギカク）

> On Friday, Jul 10, 2026 at 4:41 PM, Johannes Berg <johannes@sipsolutions.net (mailto:johannes@sipsolutions.net)> wrote:
> On Fri, 2026-07-10 at 09:21 +0200, Francesco Dolcini wrote:
> > I think we received a few patches that are validating the data received
> > from the firmware (including this one).
> >
> > I did not review any of them yet, what is your opinion on those?
> >
> > Should we consider the firmware trust-worth or should we validate
> > everything we receive from it? Is there some agreement on this topic in
> > general?
>
> I don't really know fully "in general", but I/we generally treat these
> as issues. I'm not going to worry too much about them, but I think we
> should fix them.
>
> johannes

[-- Attachment #1.2: Type: text/html, Size: 2672 bytes --]

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 885 bytes --]

Re: [PATCH] wifi: mwifiex: bound uAP association event IEs to the event buffer

[Francesco Dolcini]

On Fri, Jul 10, 2026 at 09:41:26AM +0200, Johannes Berg wrote:
> On Fri, 2026-07-10 at 09:21 +0200, Francesco Dolcini wrote:
> > I think we received a few patches that are validating the data received
> > from the firmware (including this one).
> > 
> > I did not review any of them yet, what is your opinion on those?
> > 
> > Should we consider the firmware trust-worth or should we validate
> > everything we receive from it? Is there some agreement on this topic in
> > general?
> 
> I don't really know fully "in general", but I/we generally treat these
> as issues. I'm not going to worry too much about them, but I think we
> should fix them.

Ack


On Fri, Jul 10, 2026 at 07:44:27PM +0900, skyexpoc wrote:
> > Should we consider the firmware trust-worthy or should we validate
> > everything we receive from it? Is there some agreement on this topic
> > in general?
> 
> For mwifiex there's a concrete reason not to trust it: besides the
> SDIO/PCIe parts, mwifiex also drives USB dongles (8797/8801/8997), so the
> "firmware" can sit on a hot-pluggable, potentially attacker-supplied
> device. A malformed event from such a device is the same threat model as
> any other malicious USB peripheral, not just a hypothetical firmware bug.

...

> So I'd say it's consistent with how the driver already treats
> firmware-reported lengths and agreed, worth fixing.

... and ack.


Johannes, Brian: I will try to review those patches, unless someone
gets to them earlier and they are merged.

Francesco