All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Tristan Madani <tristmd@gmail.com>
Cc: Johannes Berg <johannes@sipsolutions.net>,
	linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
	Tristan Madani <tristan@talencesecurity.com>
Subject: Re: [PATCH v3 1/6] wifi: mwifiex: fix OOB write from firmware queue_index in WMM status response
Date: Tue, 21 Apr 2026 16:19:55 -0700	[thread overview]
Message-ID: <aegGG-lGJZqbvPdJ@google.com> (raw)
In-Reply-To: <20260421134938.331334-2-tristmd@gmail.com>

Hi Tristan,

I haven't gotten through all of these yet, but so far, they seem good
aside from some cosmetic things.

Comments inline.

On Tue, Apr 21, 2026 at 01:49:33PM +0000, Tristan Madani wrote:
> From: Tristan Madani <tristan@talencesecurity.com>
> 
> The firmware-controlled queue_index (u8) from the WMM queue status TLV
> is used to index the 4-entry ac_status[] array without validation. An
> out-of-range value causes out-of-bounds writes of three firmware-
> controlled bytes into adjacent struct fields.
> 
> Add a bounds check before using queue_index as an array index.
> 
> Fixes: 5e6e3a92b9a4 ("wireless: mwifiex: initial commit for Marvell mwifiex driver")
> Signed-off-by: Tristan Madani <tristan@talencesecurity.com>
> ---
> Changes in v3:
>   - Regenerated from wireless-next with proper git format-patch to
>     produce valid index hashes (v2 had post-processed index lines).
> 
> Changes in v2:
>   - No code changes from v1.
> 
>  drivers/net/wireless/marvell/mwifiex/wmm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/wireless/marvell/mwifiex/wmm.c b/drivers/net/wireless/marvell/mwifiex/wmm.c
> index 841505e83c7fd..27e6dedcca2e8 100644
> --- a/drivers/net/wireless/marvell/mwifiex/wmm.c
> +++ b/drivers/net/wireless/marvell/mwifiex/wmm.c
> @@ -943,6 +943,11 @@ int mwifiex_ret_wmm_get_status(struct mwifiex_private *priv,
>  				    tlv_wmm_qstatus->flow_required,
>  				    tlv_wmm_qstatus->disabled);
>  
> +

Checkpatch complains about these double blank lines. Remove one.

> +			if (tlv_wmm_qstatus->queue_index >=
> +			    IEEE80211_NUM_ACS) {

Unnecessary line break.

You could also replace IEEE80211_NUM_ACS with
ARRAY_SIZE(priv->wmm.ac_status) for clarity.

> +				break;
> +			}

Might as well drop the braces, for a simple 1-liner conditional.

Also, please add a blank line after.

Thanks,
Brian

>  			ac_status = &priv->wmm.ac_status[tlv_wmm_qstatus->
>  							 queue_index];
>  			ac_status->disabled = tlv_wmm_qstatus->disabled;
> -- 
> 2.47.3
> 

  reply	other threads:[~2026-04-21 23:19 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-21 13:49 [PATCH v3 0/6] wifi: mwifiex: firmware trust boundary hardening Tristan Madani
2026-04-21 13:49 ` [PATCH v3 1/6] wifi: mwifiex: fix OOB write from firmware queue_index in WMM status response Tristan Madani
2026-04-21 23:19   ` Brian Norris [this message]
2026-04-21 13:49 ` [PATCH v3 2/6] wifi: mwifiex: fix OOB write from firmware TID in ADDBA response handler Tristan Madani
2026-04-21 23:30   ` Brian Norris
2026-04-21 13:49 ` [PATCH v3 3/6] wifi: mwifiex: fix OOB read from firmware sta_count in station list response Tristan Madani
2026-04-22 18:26   ` Brian Norris
2026-04-22 19:12     ` Johannes Berg
2026-04-22 19:54       ` Brian Norris
2026-04-22 19:57         ` Johannes Berg
2026-04-22 20:09           ` Johannes Berg
2026-04-22 19:06   ` Johannes Berg
2026-04-21 13:49 ` [PATCH v3 4/6] wifi: mwifiex: fix OOB read in scan response from mismatched TLV data sizes Tristan Madani
2026-04-22 18:28   ` Brian Norris
2026-04-21 13:49 ` [PATCH v3 5/6] wifi: mwifiex: fix OOB read from firmware intf_num in multichannel event Tristan Madani
2026-04-21 23:20   ` Brian Norris
2026-04-21 13:49 ` [PATCH v3 6/6] wifi: mwifiex: fix OOB read from inflated TLV length in IBSS peer event Tristan Madani
2026-04-21 23:20   ` Brian Norris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aegGG-lGJZqbvPdJ@google.com \
    --to=briannorris@chromium.org \
    --cc=johannes@sipsolutions.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=tristan@talencesecurity.com \
    --cc=tristmd@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.