wifi: mac80211: validate TX status rate metadata
v4 cover wifi: mac80211: validate TX status rate metadata
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 v4. The substantive rework along the review comments on v2 is unchanged since v3.
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/[email protected]/
- v1: https://lore.kernel.org/all/
0e3f97ca5cfbeb67a8e60ca5c266f4335950816b.1779619788.git.xuyq21@lenovo.com/
Changes in v4:
- Cc the blamed author and maintainer John W. Linville <[email protected]>, reported missing by netdev-ctrl (a5f69d94d8e2 is one of the blamed commits).
- Wrap the over-80-column line in the common TX status sanitizer (net/mac80211/status.c) that netdev-ctrl's checkpatch run flagged ("line length of 84 exceeds 80 columns"). No functional change.
- Wrap the overlong crash-log lines below. The v1 link is kept intact and is only split across two lines to stay within the column limit.
Review feedback from v2 and how it is addressed:
Quoted 2 lines · Johannes
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.
Quoted 6 lines
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.
Quoted 4 lines
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.
Quoted 1 line
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 | 70 ++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-)
base-commit: fefaac1176bf3cf002a8dc83339d6ed6a369941a
Signature · 1 line
2.55.0
PATCH v4 1/1 wifi: mac80211: validate TX status rate metadata
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.
--- net/mac80211/rc80211_minstrel_ht.c | 59 ++++++++++++++++++++++--- net/mac80211/status.c | 70 ++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 6 deletions(-)
net/mac80211/rc80211_minstrel_ht.c +53 -6
@@ -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 ==
net/mac80211/status.c +70 -0
88 lines not on this page · raw message
Signature · 1 line
2.55.0
The v4 changelog lists the v2 and v1 links but was missing the v3 one. For completeness:
- v3 Link: https://lore.kernel.org/all/[email protected]/
Sorry for the omission.
Thanks,
Yuqi