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

[Yuqi Xu <xuyuqiabc@gmail.com>]

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