All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/1] wifi: mac80211: validate TX status rate metadata
@ 2026-09-19  8:46 Yuqi Xu
  2026-09-19  8:46 ` [PATCH v3 1/1] " Yuqi Xu
  0 siblings, 1 reply; 2+ messages in thread
From: Yuqi Xu @ 2026-09-19  8:46 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg, Vega, Ren Wei, xuyq21

Hi Linux kernel maintainers,

minstrel_ht_tx_status() converts the HT/VHT rate reported in a TX
status into a group and rate index and uses those to index the
minstrel_ht rate tables.  Neither the common mac80211 status path nor
the minstrel validators checked that the reported MCS, NSS and
bandwidth are representable, so a malformed status report could walk
past the end of struct minstrel_rate_stats.

This is v3, reworked along the review comments on v2.

Changes in v3:
 - Move the generic rate metadata validation into the common mac80211
   TX status path (net/mac80211/status.c): the legacy ieee80211_tx_rate
   array and the rate_info based entries are sanitized before any
   status consumer runs.  This follows the review feedback that values
   such as nss == 0 are generally invalid and should not be checked by
   minstrel only.
 - Keep only the checks that depend on minstrel_ht's own tables in
   rc80211_minstrel_ht.c: the supported number of spatial streams, the
   MCS group size and the supported bandwidths.
 - Drop Cc: stable and the severity wording.  We could not identify an
   in-tree driver or firmware that reports such values; the reproducer
   uses the mac80211_hwsim userspace medium, which can only forge TX
   status for the radio it serves.  This is driver metadata hardening,
   not a report we can attribute to real hardware.
 - Restore the Assisted-by: LLM trailer that v2 dropped by mistake.
 - Rebase on wireless/main (fefaac1176bf).
 - v2: https://lore.kernel.org/all/20260529143446.1374404-1-n05ec@lzu.edu.cn/
 - v1: https://lore.kernel.org/all/0e3f97ca5cfbeb67a8e60ca5c266f4335950816b.1779619788.git.xuyq21@lenovo.com/

Review feedback from v2 and how it is addressed:

Johannes wrote:
> 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?

We agree and have reworked the description.  The only in-tree trigger
we can demonstrate is mac80211_hwsim with a userspace medium: the
medium requires 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 NSS 0 or out-of-range MCS
values, so v3 no longer asks for a stable backport.  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), which is
why v3 treats this as metadata hardening.

> 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 [...]

That is what v3 does.  ieee80211_tx_status_ext() and
ieee80211_tx_rate_update() now sanitize both the legacy
ieee80211_tx_rate array and the rate_info based entries before any
consumer runs.  minstrel_ht only keeps the checks that depend on its
own tables.

> 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 - the sanitizing does not absolve drivers.  It only makes sure
that a bad status report cannot corrupt mac80211 state.  We do not add
a WARN_ON() for the invalid values because with panic_on_warn that
would turn a driver bug into a denial of service.

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

The Assisted-by: LLM tag was dropped by mistake while reworking the
From/Signed-off-by addresses for v2.  It is restored in v3.

---- details below ----

Bug details:

minstrel_ht_tx_status() accepts both the legacy ieee80211_tx_rate array
and rate_info based entries and turns the reported HT/VHT rate into a
minstrel group and rate index.  The validation helpers only checked
that an entry was present and had tries recorded; they did not check
that the reported NSS, bandwidth and MCS values are representable by
the minstrel_ht rate tables.  A VHT entry with idx = 0x7f (NSS 8, MCS
15) produced rates[15] in a table with MCS_GROUP_RATES (10) entries and
an out-of-bounds access to struct minstrel_rate_stats.

The patch:
 - drops invalid HT/VHT entries at the mac80211 status entry points;
 - rejects rates that do not map into minstrel_ht's tables in the rate
   control algorithm itself.

Reproducer:

A self-contained program enters a user and network namespace, creates a
mac80211_hwsim radio, registers as its userspace medium, configures an
AP with an associated station, injects a radiotap VHT frame on a
monitor interface and returns the first VHT TX status with idx = 0x7f.
It is unchanged from the initial submission (the full source was
included with v1, see the link above) and is run in a KVM guest with
CONFIG_UBSAN_BOUNDS.

------BEGIN crash log------

self-contained userns setup complete; injecting radiotap VHT traffic
data TX sample: idx=0 count=1 flags=0x100 addr1=02:11:22:33:44:55
forging malformed VHT TX status: idx=0x7f flags=0x100 event=1
[    1.026837] ------------[ cut here ]------------
[    1.026841] UBSAN: array-index-out-of-bounds in net/mac80211/rc80211_minstrel_ht.c:409:33
[    1.026845] index 15 is out of range for type 'minstrel_rate_stats [10]'
[    1.026848] CPU: 1 UID: 0 PID: 24 Comm: ksoftirqd/1 Not tainted 7.3.0-rc2-00458-gfefaac1176bf #1 PREEMPT(lazy) 
[    1.026852] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-10.fc44 06/10/2025
[    1.026853] Call Trace:
[    1.026869]  <TASK>
[    1.026870]  dump_stack_lvl+0x4d/0x70
[    1.026879]  ubsan_epilogue+0x5/0x2b
[    1.026882]  __ubsan_handle_out_of_bounds.cold+0x4e/0x58
[    1.026885]  minstrel_ht_tx_status+0x98c/0xd70
[    1.026892]  ? srso_alias_return_thunk+0x5/0xfbef5
[    1.026894]  ? select_task_rq_fair+0x24d/0x1c20
[    1.026898]  rate_control_tx_status+0xac/0x130
[    1.026901]  ? ttwu_queue_wakelist+0x12d/0x260
[    1.026905]  ieee80211_tx_status_ext+0x2d5/0xc50
[    1.026908]  ? srso_alias_return_thunk+0x5/0xfbef5
[    1.026910]  ? sta_info_hash_lookup+0x95/0xd0
[    1.026913]  ieee80211_tx_status_skb+0x8a/0xc0
[    1.026916]  ieee80211_handle_queued_frames+0xb0/0xe0
[    1.026921]  ? __pfx_ieee80211_tasklet_handler+0x10/0x10
[    1.026923]  tasklet_action_common+0x159/0x260
[    1.026928]  ? tasklet_action+0xb/0x30
[    1.026929]  handle_softirqs+0xc6/0x300
[    1.026932]  ? __pfx_smpboot_thread_fn+0x10/0x10
[    1.026934]  run_ksoftirqd+0x20/0x30
[    1.026936]  smpboot_thread_fn+0xf1/0x220
[    1.026938]  kthread+0xe1/0x120
[    1.026941]  ? __pfx_kthread+0x10/0x10
[    1.026943]  ret_from_fork+0x196/0x260
[    1.026946]  ? __pfx_kthread+0x10/0x10
[    1.026947]  ? __pfx_kthread+0x10/0x10
[    1.026949]  ret_from_fork_asm+0x1a/0x30
[    1.026952]  </TASK>
[    1.026962] ---[ end trace ]---
[    1.026963] Kernel panic - not syncing: UBSAN: panic_on_warn set ...
[    1.064263] CPU: 1 UID: 0 PID: 24 Comm: ksoftirqd/1 Not tainted 7.3.0-rc2-00458-gfefaac1176bf #1 PREEMPT(lazy) 
[    1.066479] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-10.fc44 06/10/2025
[    1.068420] Call Trace:
[    1.068978]  <TASK>
[    1.069454]  dump_stack_lvl+0x4d/0x70
[    1.070288]  vpanic+0x253/0x470
[    1.071011]  panic+0x66/0x70
[    1.071694]  check_panic_on_warn.cold+0xf/0x1e
[    1.072723]  __ubsan_handle_out_of_bounds.cold+0x4e/0x58
[    1.073906]  minstrel_ht_tx_status+0x98c/0xd70
[    1.074910]  ? srso_alias_return_thunk+0x5/0xfbef5
[    1.075992]  ? select_task_rq_fair+0x24d/0x1c20
[    1.077006]  rate_control_tx_status+0xac/0x130
[    1.078002]  ? ttwu_queue_wakelist+0x12d/0x260
[    1.078997]  ieee80211_tx_status_ext+0x2d5/0xc50
[    1.080033]  ? srso_alias_return_thunk+0x5/0xfbef5
[    1.081097]  ? sta_info_hash_lookup+0x95/0xd0
[    1.082074]  ieee80211_tx_status_skb+0x8a/0xc0
[    1.083068]  ieee80211_handle_queued_frames+0xb0/0xe0
[    1.084187]  ? __pfx_ieee80211_tasklet_handler+0x10/0x10
[    1.085356]  tasklet_action_common+0x159/0x260
[    1.086348]  ? tasklet_action+0xb/0x30
[    1.087182]  handle_softirqs+0xc6/0x300
[    1.088053]  ? __pfx_smpboot_thread_fn+0x10/0x10
[    1.089129]  run_ksoftirqd+0x20/0x30
[    1.089930]  smpboot_thread_fn+0xf1/0x220
[    1.090835]  kthread+0xe1/0x120
[    1.091547]  ? __pfx_kthread+0x10/0x10
[    1.092393]  ret_from_fork+0x196/0x260
[    1.093242]  ? __pfx_kthread+0x10/0x10
[    1.094090]  ? __pfx_kthread+0x10/0x10
[    1.094927]  ret_from_fork_asm+0x1a/0x30
[    1.095817]  </TASK>
[    1.096458] Kernel Offset: 0x2e400000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
[    1.099071] Rebooting in 1 seconds..

------END crash log-----

Best regards,
Yuqi Xu

Yuqi Xu (1):
  wifi: mac80211: validate TX status rate metadata

 net/mac80211/rc80211_minstrel_ht.c | 59 ++++++++++++++++++++++---
 net/mac80211/status.c              | 69 ++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 6 deletions(-)


base-commit: fefaac1176bf3cf002a8dc83339d6ed6a369941a
-- 
2.55.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* [PATCH v3 1/1] wifi: mac80211: validate TX status rate metadata
  2026-09-19  8:46 [PATCH v3 0/1] wifi: mac80211: validate TX status rate metadata Yuqi Xu
@ 2026-09-19  8:46 ` Yuqi Xu
  0 siblings, 0 replies; 2+ messages in thread
From: Yuqi Xu @ 2026-09-19  8:46 UTC (permalink / raw)
  To: linux-wireless; +Cc: Johannes Berg, Vega, Ren Wei, xuyq21

minstrel_ht_tx_status() converts the HT/VHT rate reported in a TX status
into a group and rate index and uses those to index the minstrel_ht rate
tables.  The existing validation only checked that an entry was present
and had tries recorded; it did not check that the reported MCS, spatial
stream count and bandwidth are representable by the tables, so malformed
metadata passed to minstrel_ht_get_stats() or
minstrel_ht_ri_get_stats() could be used as an out-of-bounds index.

Validate the rate metadata in the common TX status path instead of only
in minstrel_ht, since the values are used by more than rate control.
The legacy ieee80211_tx_rate array and the rate_info based entries are
sanitized in ieee80211_tx_status_ext() and ieee80211_tx_rate_update()
before any consumer runs: an entry that does not describe a valid rate
for its encoding is dropped.  minstrel_ht keeps the checks that depend
on its own tables, i.e. the supported number of spatial streams, the MCS
group size and the supported bandwidths.

This does not take away the driver's responsibility to report sane
values, it only prevents a single bad report from corrupting mac80211
state.  No WARN_ON() is added: with panic_on_warn a driver bug would
otherwise turn into a denial of service.

Fixes: a5f69d94d8e2 ("mac80211: Get rid of search loop for rate group index")
Fixes: 9208247d74bc ("mac80211: minstrel_ht: add basic support for VHT rates <= 3SS@80MHz")
Reported-by: Vega <vega@nebusec.ai>
Assisted-by: LLM
Signed-off-by: Yuqi Xu <xuyuqiabc@gmail.com>
Reviewed-by: Ren Wei <weir@nebusec.ai>
---
 net/mac80211/rc80211_minstrel_ht.c | 59 ++++++++++++++++++++++---
 net/mac80211/status.c              | 69 ++++++++++++++++++++++++++++++
 2 files changed, 122 insertions(+), 6 deletions(-)

diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index b73ef3adfcc5..60f3e4c8676d 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -1193,6 +1193,54 @@ minstrel_ht_update_stats(struct minstrel_priv *mp, struct minstrel_ht_sta *mi)
 	mi->sample_time = jiffies;
 }
 
+/*
+ * Check whether an HT/VHT rate from a TX status entry maps to an entry
+ * in the minstrel_ht rate tables.  Values that cannot be represented
+ * there must not be used for indexing mi->groups[] and the MCS groups.
+ */
+static bool
+minstrel_ht_txstat_rate_valid(struct ieee80211_tx_rate *rate)
+{
+	unsigned int bw;
+
+	if (!(rate->flags & (IEEE80211_TX_RC_MCS | IEEE80211_TX_RC_VHT_MCS)))
+		return true;
+
+	if (rate->flags & IEEE80211_TX_RC_MCS) {
+		/* minstrel_ht supports up to MINSTREL_MAX_STREAMS streams */
+		return rate->idx < MINSTREL_MAX_STREAMS * 8;
+	}
+
+	/* minstrel_ht has no VHT groups for 160 MHz and wider */
+	bw = !!(rate->flags & IEEE80211_TX_RC_40_MHZ_WIDTH) +
+	     2 * !!(rate->flags & IEEE80211_TX_RC_80_MHZ_WIDTH);
+	if ((rate->flags & IEEE80211_TX_RC_160_MHZ_WIDTH) || bw > BW_80)
+		return false;
+
+	return ieee80211_rate_get_vht_nss(rate) <= MINSTREL_MAX_STREAMS &&
+	       ieee80211_rate_get_vht_mcs(rate) < MCS_GROUP_RATES;
+}
+
+static bool
+minstrel_ht_ri_txstat_rate_valid(struct rate_info *rate)
+{
+	if (!(rate->flags & (RATE_INFO_FLAGS_MCS | RATE_INFO_FLAGS_VHT_MCS)))
+		return true;
+
+	if (rate->flags & RATE_INFO_FLAGS_MCS) {
+		/* minstrel_ht supports up to MINSTREL_MAX_STREAMS streams */
+		return rate->mcs < MINSTREL_MAX_STREAMS * 8;
+	}
+
+	/* minstrel_ht has VHT groups only for 20/40/80 MHz */
+	if (rate->bw != RATE_INFO_BW_20 && rate->bw != RATE_INFO_BW_40 &&
+	    rate->bw != RATE_INFO_BW_80)
+		return false;
+
+	return rate->nss <= MINSTREL_MAX_STREAMS &&
+	       rate->mcs < MCS_GROUP_RATES;
+}
+
 static bool
 minstrel_ht_txstat_valid(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 			 struct ieee80211_tx_rate *rate)
@@ -1205,9 +1253,8 @@ 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)
-		return true;
+	if (rate->flags & (IEEE80211_TX_RC_MCS | IEEE80211_TX_RC_VHT_MCS))
+		return minstrel_ht_txstat_rate_valid(rate);
 
 	for (i = 0; i < ARRAY_SIZE(mp->cck_rates); i++)
 		if (rate->idx == mp->cck_rates[i])
@@ -1235,9 +1282,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)
-		return true;
+	if (rate_status->rate_idx.flags &
+	    (RATE_INFO_FLAGS_MCS | RATE_INFO_FLAGS_VHT_MCS))
+		return minstrel_ht_ri_txstat_rate_valid(&rate_status->rate_idx);
 
 	for (i = 0; i < ARRAY_SIZE(mp->cck_rates); i++) {
 		if (rate_status->rate_idx.legacy ==
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 3d811f652603..1730d48926e7 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -1164,6 +1164,72 @@ void ieee80211_tx_status_skb(struct ieee80211_hw *hw, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(ieee80211_tx_status_skb);
 
+/*
+ * Check whether the HT/VHT rate information in a TX status entry is
+ * valid for its encoding.  This is not specific to any rate control
+ * algorithm; all status consumers rely on the values being sane.
+ */
+static bool ieee80211_tx_status_rate_info_valid(const struct rate_info *rate)
+{
+	if (rate->flags & RATE_INFO_FLAGS_MCS)
+		return rate->mcs < 32;
+
+	if (rate->flags & RATE_INFO_FLAGS_VHT_MCS)
+		return rate->nss >= 1 && rate->nss <= 8 && rate->mcs <= 11;
+
+	return true;
+}
+
+static bool
+ieee80211_tx_status_tx_rate_valid(const struct ieee80211_tx_rate *rate)
+{
+	if (rate->flags & IEEE80211_TX_RC_MCS)
+		return rate->idx < 32;
+
+	if (rate->flags & IEEE80211_TX_RC_VHT_MCS)
+		return ieee80211_rate_get_vht_mcs(rate) <= 11;
+
+	return true;
+}
+
+/*
+ * Drop TX status rate entries that don't describe a valid rate.  The
+ * status information is used by rate control and by other mac80211
+ * code, and a malformed entry must not be able to corrupt state beyond
+ * the driver that reported it.
+ */
+static void
+ieee80211_tx_status_drop_invalid_rates(struct ieee80211_tx_status *status)
+{
+	int i;
+
+	for (i = 0; i < status->n_rates; i++) {
+		if (ieee80211_tx_status_rate_info_valid(&status->rates[i].rate_idx))
+			continue;
+
+		status->rates[i].try_count = 0;
+		memset(&status->rates[i].rate_idx, 0,
+		       sizeof(status->rates[i].rate_idx));
+	}
+
+	if (!status->info)
+		return;
+
+	for (i = 0; i < IEEE80211_TX_MAX_RATES; i++) {
+		struct ieee80211_tx_rate *rate;
+
+		rate = &status->info->status.rates[i];
+		if (rate->idx < 0)
+			break;
+
+		if (ieee80211_tx_status_tx_rate_valid(rate))
+			continue;
+
+		rate->idx = -1;
+		rate->count = 0;
+	}
+}
+
 void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 			     struct ieee80211_tx_status *status)
 {
@@ -1176,6 +1242,8 @@ void ieee80211_tx_status_ext(struct ieee80211_hw *hw,
 	bool acked, noack_success, ack_signal_valid;
 	u16 tx_time_est;
 
+	ieee80211_tx_status_drop_invalid_rates(status);
+
 	if (pubsta) {
 		sta = container_of(pubsta, struct sta_info, sta);
 
@@ -1300,6 +1368,7 @@ void ieee80211_tx_rate_update(struct ieee80211_hw *hw,
 		.sta = pubsta,
 	};
 
+	ieee80211_tx_status_drop_invalid_rates(&status);
 	rate_control_tx_status(local, &status);
 
 	if (ieee80211_hw_check(&local->hw, HAS_RATE_CONTROL))
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-09-19  8:46 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-19  8:46 [PATCH v3 0/1] wifi: mac80211: validate TX status rate metadata Yuqi Xu
2026-09-19  8:46 ` [PATCH v3 1/1] " Yuqi Xu

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.