Re: [PATCH v2] wifi: mwifiex: validate action frame fixed fields

[Francesco Dolcini]

On Thu, Jul 23, 2026 at 09:10:13AM +0800, Zhao Li wrote:
> mwifiex_process_mgmt_packet() accepts an rx_pkt_length as small as a
> four-address struct ieee80211_hdr plus the two-byte firmware length prefix.
> After stripping the prefix, mwifiex_parse_mgmt_packet() can receive a
> buffer equal to sizeof(struct ieee80211_hdr).
> 
> For action frames, the parser reads the category immediately after that
> header and, for a public action frame, reads the following action code
> without verifying that either field is present. A minimal frame therefore
> reads one or two bytes beyond the RX buffer.
> 
> Validate the header and require the category and public action-code fields
> before reading them. mwifiex parses the firmware four-address layout before
> removing addr4, so add ETH_ALEN to the standard
> IEEE80211_MIN_ACTION_SIZE() offsets.
> 
> Suggested-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> Fixes: 72e5aa8d2a6d ("mwifiex: support for parsing TDLS discovery frames")
> Cc: stable@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/all/66f148d83eb9f0970b9abbccc85d1b61244e54ad.camel@xxxxxxxxxxxxxxxx/
> Link: https://lore.kernel.org/all/20260708195911.84365-8-enderaoelyther@xxxxxxxxx/
> Assisted-by: Codex:gpt-5
> Assisted-by: Claude:opus-4.8
> Signed-off-by: Zhao Li <enderaoelyther@xxxxxxxxx>
> ---
> Changes in v2:
>   - Add a local header check and express the action field sizes with
>     IEEE80211_MIN_ACTION_SIZE(), accounting for the firmware layout.
> 
>  drivers/net/wireless/marvell/mwifiex/util.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/util.c b/drivers/net/wireless/marvell/mwifiex/util.c
> index 7d3631d212236..4043590137391 100644
> --- a/drivers/net/wireless/marvell/mwifiex/util.c
> +++ b/drivers/net/wireless/marvell/mwifiex/util.c
> @@ -313,13 +313,22 @@ mwifiex_parse_mgmt_packet(struct mwifiex_private *priv, u8 *payload, u16 len,
>  	u8 category, action_code, *addr2;
>  	struct ieee80211_hdr *ieee_hdr = (void *)payload;
>  
> +	if (len < sizeof(*ieee_hdr))
> +		return -1;
> +

I am confused, why is this check needed here, and not done in
mwifiex_process_mgmt_packet()?