net/mac80211/cfg.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
VHT operating mode notifications must not be processed when the channel
width is 5 MHz or 10 MHz, as the VHT specification does not support these
narrow widths.
Without validation, a malformed notification using 10 MHz can reach
ieee80211_chan_width_to_rx_bw(), triggering a WARN_ON due to the invalid
width. This issue was reported by syzbot.
Reject these widths early in sta_link_apply_parameters() when
opmode_notif is used.
Reported-by: syzbot+ededba317ddeca8b3f08@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ededba317ddeca8b3f08
Fixes: 751e7489c1d7 ("wifi: mac80211: expose ieee80211_chan_width_to_rx_bw() to drivers")
Tested-by: syzbot+ededba317ddeca8b3f08@syzkaller.appspotmail.com
Signed-off-by: Moon Hee Lee <moonhee.lee.ca@gmail.com>
---
net/mac80211/cfg.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 56540c3701ed..5a6ae093a8bd 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1981,6 +1981,21 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
ieee80211_sta_init_nss(link_sta);
if (params->opmode_notif_used) {
+ enum nl80211_chan_width width = link->conf->chanreq.oper.width;
+
+ switch (width) {
+ case NL80211_CHAN_WIDTH_20_NOHT:
+ case NL80211_CHAN_WIDTH_20:
+ case NL80211_CHAN_WIDTH_40:
+ case NL80211_CHAN_WIDTH_80:
+ case NL80211_CHAN_WIDTH_160:
+ case NL80211_CHAN_WIDTH_80P80:
+ case NL80211_CHAN_WIDTH_320:
+ break;
+ default:
+ return -EINVAL;
+ }
+
/* returned value is only needed for rc update, but the
* rc isn't initialized here yet, so ignore it
*/
--
2.43.0
On Wed Jul 2, 2025 at 8:59 AM CEST, Moon Hee Lee wrote: > VHT operating mode notifications must not be processed when the channel > width is 5 MHz or 10 MHz, as the VHT specification does not support these > narrow widths. Hello, Is this really specific for VHT ? or for HE /EHT as well ? > > Without validation, a malformed notification using 10 MHz can reach > ieee80211_chan_width_to_rx_bw(), triggering a WARN_ON due to the invalid > width. This issue was reported by syzbot. > > Reject these widths early in sta_link_apply_parameters() when > opmode_notif is used. > > Reported-by: syzbot+ededba317ddeca8b3f08@syzkaller.appspotmail.com > Closes: https://syzkaller.appspot.com/bug?extid=ededba317ddeca8b3f08 > Fixes: 751e7489c1d7 ("wifi: mac80211: expose ieee80211_chan_width_to_rx_bw() to drivers") > Tested-by: syzbot+ededba317ddeca8b3f08@syzkaller.appspotmail.com > Signed-off-by: Moon Hee Lee <moonhee.lee.ca@gmail.com> > --- > net/mac80211/cfg.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c > index 56540c3701ed..5a6ae093a8bd 100644 > --- a/net/mac80211/cfg.c > +++ b/net/mac80211/cfg.c > @@ -1981,6 +1981,21 @@ static int sta_link_apply_parameters(struct ieee80211_local *local, > ieee80211_sta_init_nss(link_sta); > > if (params->opmode_notif_used) { > + enum nl80211_chan_width width = link->conf->chanreq.oper.width; > + > + switch (width) { > + case NL80211_CHAN_WIDTH_20_NOHT: Because this seems weird for VHT > + case NL80211_CHAN_WIDTH_20: > + case NL80211_CHAN_WIDTH_40: > + case NL80211_CHAN_WIDTH_80: > + case NL80211_CHAN_WIDTH_160: > + case NL80211_CHAN_WIDTH_80P80: > + case NL80211_CHAN_WIDTH_320: And this did not exist for VHT either > + break; > + default: > + return -EINVAL; > + } > + > /* returned value is only needed for rc update, but the > * rc isn't initialized here yet, so ignore it > */
Hi Nicolas, On Thu, Jul 3, 2025 at 1:12 AM Nicolas Escande <nico.escande@gmail.com> wrote: > Is this really specific for VHT ? or for HE /EHT as well ? > > > + switch (width) { > > + case NL80211_CHAN_WIDTH_20_NOHT: > Because this seems weird for VHT > > + case NL80211_CHAN_WIDTH_320: > And this did not exist for VHT either > Thanks for the feedback. The intention was to handle VHT opmode notifications, as noted in the commit message, but the check incorrectly included widths that are not valid for VHT, such as 20_NOHT and 320. I will update v2 to reject any invalid widths, not just 5 or 10 MHz, and restrict the check to the valid set for VHT: 20, 40, 80, 160, and 80+80. Best regards, Moonhee
On Thu, 2025-07-03 at 02:02 -0700, Moonhee Lee wrote: > Hi Nicolas, > > On Thu, Jul 3, 2025 at 1:12 AM Nicolas Escande <nico.escande@gmail.com> wrote: > > > Is this really specific for VHT ? or for HE /EHT as well ? > > > > > + switch (width) { > > > + case NL80211_CHAN_WIDTH_20_NOHT: > > Because this seems weird for VHT > > > + case NL80211_CHAN_WIDTH_320: > > And this did not exist for VHT either Yes, but see below. > > Thanks for the feedback. The intention was to handle VHT opmode notifications, > as noted in the commit message, but the check incorrectly included widths that > are not valid for VHT, such as 20_NOHT and 320. I will update v2 to reject any > invalid widths, not just 5 or 10 MHz, and restrict the check to the valid set > for VHT: 20, 40, 80, 160, and 80+80. I'm not entirely sure that'd be correct. 320 MHz can only be used on the 6 GHz band, so clients must be at least HE, but I'm not sure that VHT opmode notification frames are completely illegal for them, even if they'd like use OMI instead. How did syzbot even manage to get a 10 MHz thing running though? johannes
On Thu, 2025-07-03 at 17:09 +0200, Johannes Berg wrote: > > How did syzbot even manage to get a 10 MHz thing running though? > n/m, it just did that quite explicitly. I really want to get rid of all that 5/10 MHz code, but we still have S1G so that might still happen anyway, and I expect for S1G VHT operating mode notification frames _are_ invalid. johannes
Hi Johannes, On Thu, Jul 3, 2025 at 8:09 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > I'm not entirely sure that'd be correct. 320 MHz can only be used on the > 6 GHz band, so clients must be at least HE, but I'm not sure that VHT > opmode notification frames are completely illegal for them, even if > they'd like use OMI instead. > Understood. That clarifies the case for not filtering out 320 MHz, even when a VHT opmode notification is present. On Thu, Jul 3, 2025 at 8:11 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > n/m, it just did that quite explicitly. I really want to get rid of all > that 5/10 MHz code, but we still have S1G so that might still happen > anyway, and I expect for S1G VHT operating mode notification frames > _are_ invalid. To address this, I plan to reject the set of channel widths that are not valid with VHT opmode. This includes all channel widths below 20 MHz, as well as 20 MHz without HT (20_NOHT), which is also incompatible. Would the following logic for v2 be acceptable? /* reject channel widths not valid with VHT opmode */ switch (width) { case NL80211_CHAN_WIDTH_5: case NL80211_CHAN_WIDTH_10: case NL80211_CHAN_WIDTH_1: case NL80211_CHAN_WIDTH_2: case NL80211_CHAN_WIDTH_4: case NL80211_CHAN_WIDTH_8: case NL80211_CHAN_WIDTH_16: case NL80211_CHAN_WIDTH_20_NOHT: return -EINVAL; default: break; } This allows valid HT/VHT channel widths, including 20, 40, 80, 80+80, 160, and 320, to pass through while filtering out values that would otherwise trigger a WARN_ON. Best regards, Moonhee
On Thu, 2025-07-03 at 09:35 -0700, Moonhee Lee wrote: > > To address this, I plan to reject the set of channel widths that are not valid > with VHT opmode. This includes all channel widths below 20 MHz, as well as > 20 MHz without HT (20_NOHT), which is also incompatible. > > Would the following logic for v2 be acceptable? > > /* reject channel widths not valid with VHT opmode */ > switch (width) { > case NL80211_CHAN_WIDTH_5: > case NL80211_CHAN_WIDTH_10: > case NL80211_CHAN_WIDTH_1: > case NL80211_CHAN_WIDTH_2: > case NL80211_CHAN_WIDTH_4: > case NL80211_CHAN_WIDTH_8: > case NL80211_CHAN_WIDTH_16: > case NL80211_CHAN_WIDTH_20_NOHT: > return -EINVAL; > default: > break; > } > > This allows valid HT/VHT channel widths, including 20, 40, 80, 80+80, 160, and > 320, to pass through while filtering out values that would otherwise trigger a > WARN_ON. I think it'd make more sense to go the other way around and list the bandwidths that are _valid_ here, even if I don't see it getting extended any time soon (anyone working on TVHT? ;-) ) But in some way I also have a feeling we _should_ be able to reject this in cfg80211 already - although it seems that right now we cannot. Hmm. I guess better to have this validation here now than fail/WARN, but then I'd like a positive list of allowed values, rather than forbidden ones. johannes
On Thu, Jul 3, 2025 at 9:54 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > I think it'd make more sense to go the other way around and list the > bandwidths that are _valid_ here, even if I don't see it getting > extended any time soon (anyone working on TVHT? ;-) ) > > But in some way I also have a feeling we _should_ be able to reject this > in cfg80211 already - although it seems that right now we cannot. Hmm. I > guess better to have this validation here now than fail/WARN, but then > I'd like a positive list of allowed values, rather than forbidden ones. Thank you for the feedback. I agree it is clearer to apply a positive list of valid channel widths rather than excluding specific ones. I will update v2 accordingly, and drop NL80211_CHAN_WIDTH_20_NOHT from the current patch as well. This ensures only the valid set is accepted, while preventing the WARN_ON from being triggered. Best regards, Moonhee
© 2016 - 2025 Red Hat, Inc.