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

[Francesco Dolcini <francesco@dolcini.it>]

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,

Why not validating event->len instead, when we receive an MWIFIEX_TYPE_EVENT
from the firmware? we could just add a new, validated, u16 event_len, in struct
mwifiex_adapter and after that we can just use it in the code.

and we reject invalid events as soon as possible in the software.


> 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;
> +				}

In case we want to keep the change here, I would ask you to make the
code slighly more compact.

just define a

evt_len = le16_to_cpu(event->len)

and use it.