[PATCH v2 1/1] wifi: mac80211: validate minstrel tx status rates

[PATCH v2 1/1] wifi: mac80211: validate minstrel tx status rates

[Ren Wei]

From: Yuqi Xu <xuyuqiabc@gmail.com>

minstrel_ht_tx_status() accepts both legacy tx status entries and
rate_info-based status entries, then turns the reported HT/VHT rate
into a minstrel group and rate index.

The validation helpers only checked that the entry was present and had
tries recorded. They did not verify that the reported HT stream count,
VHT NSS, VHT bandwidth encoding, and MCS value were representable by
minstrel_ht's rate tables. As a result, malformed tx status metadata
could produce group or rate indices outside the tables that
minstrel_ht_get_stats() and minstrel_ht_ri_get_stats() index.

Teach the existing tx status validation path to enforce the exact
constraints used by minstrel HT's tables for both status formats.
Reject HT rates beyond the supported stream groups, and reject VHT
rates with unsupported bandwidth encodings, invalid NSS values, or
MCS values outside the table.

This keeps the fix at the existing trust boundary and leaves the
stats lookup path unchanged.

Fixes: ec8aa669b839 ("mac80211: add the minstrel_ht rate control algorithm")
Cc: stable@kernel.org
Reported-by: Yuan Tan <yuantan098@gmail.com>
Reported-by: Zhengchuan Liang <zcliangcn@gmail.com>
Reported-by: Xin Liu <bird@lzu.edu.cn>
Signed-off-by: Yuqi Xu <xuyuqiabc@gmail.com>
Signed-off-by: Ren Wei <n05ec@lzu.edu.cn>
---
Changes in v2:
- shorten the subject line
- align the From/Signed-off-by address
- drop the Assisted-by tag
- v1 Link: https://lore.kernel.org/all/0e3f97ca5cfbeb67a8e60ca5c266f4335950816b.1779619788.git.xuyq21@lenovo.com/

 net/mac80211/rc80211_minstrel_ht.c | 47 +++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index b73ef3adfcc5..a35d42c77ac5 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -323,6 +323,43 @@ minstrel_ht_is_legacy_group(int group)
 	       group == MINSTREL_OFDM_GROUP;
 }
 
+static bool
+minstrel_ht_txstat_valid_rate(struct ieee80211_tx_rate *rate)
+{
+	unsigned int bw;
+
+	if (rate->flags & IEEE80211_TX_RC_MCS)
+		return rate->idx < MINSTREL_MAX_STREAMS * 8;
+
+	if (!(rate->flags & IEEE80211_TX_RC_VHT_MCS))
+		return true;
+
+	bw = !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH) +
+	     2 * !!(rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH);
+
+	return !(rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH) &&
+	       bw <= BW_80 &&
+	       ieee80211_rate_get_vht_nss(rate) <= MINSTREL_MAX_STREAMS &&
+	       ieee80211_rate_get_vht_mcs(rate) < MCS_GROUP_RATES;
+}
+
+static bool
+minstrel_ht_ri_txstat_valid_rate(struct rate_info *rate)
+{
+	if (rate->flags & RATE_INFO_FLAGS_MCS)
+		return rate->mcs < MINSTREL_MAX_STREAMS * 8;
+
+	if (!(rate->flags & RATE_INFO_FLAGS_VHT_MCS))
+		return true;
+
+	return (rate->bw == RATE_INFO_BW_20 ||
+		rate->bw == RATE_INFO_BW_40 ||
+		rate->bw == RATE_INFO_BW_80) &&
+	       rate->nss >= 1 &&
+	       rate->nss <= MINSTREL_MAX_STREAMS &&
+	       rate->mcs < MCS_GROUP_RATES;
+}
+
 /*
  * Look up an MCS group index based on mac80211 rate information
  */
@@ -1205,8 +1242,9 @@ minstrel_ht_txstat_valid(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	if (!rate->count)
 		return false;
 
-	if (rate->flags & IEEE80211_TX_RC_MCS ||
-	    rate->flags & IEEE80211_TX_RC_VHT_MCS)
+	if ((rate->flags & IEEE80211_TX_RC_MCS ||
+	     rate->flags & IEEE80211_TX_RC_VHT_MCS) &&
+	    minstrel_ht_txstat_valid_rate(rate))
 		return true;
 
 	for (i = 0; i < ARRAY_SIZE(mp->cck_rates); i++)
@@ -1235,8 +1273,9 @@ minstrel_ht_ri_txstat_valid(struct minstrel_priv *mp,
 	if (!rate_status->try_count)
 		return false;
 
-	if (rate_status->rate_idx.flags & RATE_INFO_FLAGS_MCS ||
-	    rate_status->rate_idx.flags & RATE_INFO_FLAGS_VHT_MCS)
+	if ((rate_status->rate_idx.flags & RATE_INFO_FLAGS_MCS ||
+	     rate_status->rate_idx.flags & RATE_INFO_FLAGS_VHT_MCS) &&
+	    minstrel_ht_ri_txstat_valid_rate(&rate_status->rate_idx))
 		return true;
 
 	for (i = 0; i < ARRAY_SIZE(mp->cck_rates); i++) {
-- 
2.54.0

Re: [PATCH v2 1/1] wifi: mac80211: validate minstrel tx status rates

[Johannes Berg]

On Fri, 2026-05-29 at 22:34 +0800, Ren Wei wrote:
> From: Yuqi Xu <xuyuqiabc@gmail.com>
> 
> minstrel_ht_tx_status() accepts both legacy tx status entries and
> rate_info-based status entries, then turns the reported HT/VHT rate
> into a minstrel group and rate index.
> 
> The validation helpers only checked that the entry was present and had
> tries recorded. They did not verify that the reported HT stream count,
> VHT NSS, VHT bandwidth encoding, and MCS value were representable by
> minstrel_ht's rate tables. As a result, malformed tx status metadata
> could produce group or rate indices outside the tables that
> minstrel_ht_get_stats() and minstrel_ht_ri_get_stats() index.
> 
> Teach the existing tx status validation path to enforce the exact
> constraints used by minstrel HT's tables for both status formats.
> Reject HT rates beyond the supported stream groups, and reject VHT
> rates with unsupported bandwidth encodings, invalid NSS values, or
> MCS values outside the table.
> 
> This keeps the fix at the existing trust boundary and leaves the
> stats lookup path unchanged.

So ... I'm not really very happy with this.

First of all, I think it's kind of overblown with the CC stable etc.,
can you actually find a situation where a driver reports such a thing?

Secondly, I don't think it's the right place to be checking, we use the
data a lot for other things in status handling, and might expand that,
so it seems that instead of having specific checks for minstrel, we
should have most of the checks in general (e.g. nss==0 is generally
invalid), and only have the things that matter for minstrel specifically
(say bandwidth) there - if those even matter, because surely we don't
expect a device that supports 80 MHz to start reporting 320 MHz, and if
that really seems important maybe we should just validate that elsewhere
as well.

But I also think that if this stuff really comes from firmware rather
than being built by the driver, it should be the driver's responsibility
to not report nonsense. I doubt we can protect against any driver
nonsense here in mac80211.

> Changes in v2:
> - shorten the subject line
> - align the From/Signed-off-by address
> - drop the Assisted-by tag

Why drop it now, when before you were saying it was?

johannes

Re: [PATCH v2 1/1] wifi: mac80211: validate minstrel tx status rates

[Yuqi Xu]

Hi Johannes,

thanks for the review, and sorry for the long turnaround.  I have
reworked the patch along your comments and sent v3:

  https://lore.kernel.org/all/cover.1789801378.git.xuyuqiabc@gmail.com/

Replies inline below.

On Tue, 2026-06-02 at 15:26 +0200, Johannes Berg wrote:
> So ... I'm not really very happy with this.
>
> First of all, I think it's kind of overblown with the CC stable etc.,
> can you actually find a situation where a driver reports such a thing?

You are right, and we could not find one.  The only in-tree path we can
demonstrate is mac80211_hwsim together with a userspace medium: the
medium has to register with the driver (CAP_NET_ADMIN in a user and
network namespace) and can then return forged TX status for the radio
it serves.  We are not aware of any in-tree driver or firmware
reporting an NSS of 0 or an out-of-range MCS index, and driver-side
work is validating the same kind of metadata before it reaches
mac80211 (e.g. the proposed "[PATCH wireless v2] wifi: mt76: mt76x02:
validate TX-status rate index before mac80211 handoff", 2026-08-26).
v3 therefore drops Cc: stable and no longer describes this as a
vulnerability; it is defensive hardening of the driver metadata path.

> Secondly, I don't think it's the right place to be checking, we use
> the data a lot for other things in status handling, and might expand
> that, so it seems that instead of having specific checks for
> minstrel, we should have most of the checks in general (e.g. nss==0
> is generally invalid), and only have the things that matter for
> minstrel specifically (say bandwidth) there - if those even matter,
> because surely we don't expect a device that supports 80 MHz to start
> reporting 320 MHz, and if that really seems important maybe we should
> just validate that elsewhere as well.

That is exactly what v3 does now.  The generic checks live in
net/mac80211/status.c and run for every TX status before any consumer
sees it: ieee80211_tx_status_ext() and ieee80211_tx_rate_update()
sanitize both the legacy ieee80211_tx_rate array and the rate_info
based entries (HT MCS 0..31, VHT NSS 1..8, VHT MCS 0..11).  Entries
that do not describe a valid rate are dropped, so rate control,
statistics and radiotap all see the same sane data.  minstrel_ht only
keeps the checks that depend on its own tables: the number of spatial
streams, the supported bandwidth (no VHT groups for 160 MHz and wider)
and the MCS range must map to a table entry.

> But I also think that if this stuff really comes from firmware rather
> than being built by the driver, it should be the driver's
> responsibility to not report nonsense.  I doubt we can protect
> against any driver nonsense here in mac80211.

Agreed, this is not meant to take that responsibility away from
drivers.  The point is only that one bad status report must not corrupt
mac80211 state, no matter which driver or firmware produced it.  We
deliberately do not add a WARN_ON() for the invalid values: with
panic_on_warn that would turn a driver bug into a denial of service,
and the values are not something mac80211 can act on anyway.

> > Changes in v2:
> > - shorten the subject line
> > - align the From/Signed-off-by address
> > - drop the Assisted-by tag
>
> Why drop it now, when before you were saying it was?

That was a mistake on our side while reworking the From/Signed-off-by
addresses for v2.  The Assisted-by: LLM tag is part of our workflow
documentation for LLM-assisted patches and is restored in v3.

Thanks,
Yuqi Xu