Stations can update bandwidth/NSS change in
VHT action frame with action type Operating Mode Notification.
(IEEE Std 802.11-2020 - 9.4.1.53 Operating Mode field)
For Operating Mode Notification, an RX NSS change to a value
greater than AP's maximum NSS should not be allowed.
Hence allow NSS change only up to maximum NSS that is negotiated
and capped to AP's capability during association.
Signed-off-by: Rameshkumar Sundaram <[email protected]>
---
net/mac80211/vht.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/net/mac80211/vht.c b/net/mac80211/vht.c
index c804890dc623..fb1c55109884 100644
--- a/net/mac80211/vht.c
+++ b/net/mac80211/vht.c
@@ -623,7 +623,7 @@ u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
enum ieee80211_sta_rx_bandwidth new_bw;
struct sta_opmode_info sta_opmode = {};
u32 changed = 0;
- u8 nss;
+ u8 nss, cur_nss;
/* ignore - no support for BF yet */
if (opmode & IEEE80211_OPMODE_NOTIF_RX_NSS_TYPE_BF)
@@ -634,10 +634,22 @@ u32 __ieee80211_vht_handle_opmode(struct ieee80211_sub_if_data *sdata,
nss += 1;
if (link_sta->pub->rx_nss != nss) {
- link_sta->pub->rx_nss = nss;
- sta_opmode.rx_nss = nss;
- changed |= IEEE80211_RC_NSS_CHANGED;
- sta_opmode.changed |= STA_OPMODE_N_SS_CHANGED;
+ cur_nss = link_sta->pub->rx_nss;
+ link_sta->pub->rx_nss = 0;
+ ieee80211_sta_set_rx_nss(link_sta);
+ /* Do not allow an nss change to rx_nss greater than max_nss
+ * negotiated and capped to APs capability during association.
+ */
+ if (nss <= link_sta->pub->rx_nss) {
+ link_sta->pub->rx_nss = nss;
+ sta_opmode.rx_nss = nss;
+ changed |= IEEE80211_RC_NSS_CHANGED;
+ sta_opmode.changed |= STA_OPMODE_N_SS_CHANGED;
+ } else {
+ link_sta->pub->rx_nss = cur_nss;
+ pr_warn_ratelimited("Ignoring NSS change in VHT Operating Mode Notification from %pM with invalid nss %d",
+ link_sta->pub->addr, nss);
+ }
}
switch (opmode & IEEE80211_OPMODE_NOTIF_CHANWIDTH_MASK) {
--
2.25.1
On Tue, 2022-10-18 at 10:13 +0530, Rameshkumar Sundaram wrote:
> Stations can update bandwidth/NSS change in
> VHT action frame with action type Operating Mode Notification.
> (IEEE Std 802.11-2020 - 9.4.1.53 Operating Mode field)
>
> For Operating Mode Notification, an RX NSS change to a value
> greater than AP's maximum NSS should not be allowed.
> Hence allow NSS change only up to maximum NSS that is negotiated
> and capped to AP's capability during association.
That seems reasonable. Might be worth noting in a comment in the code
which AP has such bugs though, just so we know it later.
> if (link_sta->pub->rx_nss != nss) {
> - link_sta->pub->rx_nss = nss;
> - sta_opmode.rx_nss = nss;
> - changed |= IEEE80211_RC_NSS_CHANGED;
> - sta_opmode.changed |= STA_OPMODE_N_SS_CHANGED;
> + cur_nss = link_sta->pub->rx_nss;
> + link_sta->pub->rx_nss = 0;
> + ieee80211_sta_set_rx_nss(link_sta);
> + /* Do not allow an nss change to rx_nss greater than max_nss
> + * negotiated and capped to APs capability during association.
> + */
> + if (nss <= link_sta->pub->rx_nss) {
> + link_sta->pub->rx_nss = nss;
That, however, doesn't seem right. It means that you can only ever
reduce the RX NSS, not switch it around within the originally negotiated
range.
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Friday, October 21, 2022 5:13 PM
> To: Rameshkumar Sundaram (QUIC) <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH] wifi: mac80211: Allow NSS change only up to capability
>
> On Tue, 2022-10-18 at 10:13 +0530, Rameshkumar Sundaram wrote:
> > Stations can update bandwidth/NSS change in VHT action frame with
> > action type Operating Mode Notification.
> > (IEEE Std 802.11-2020 - 9.4.1.53 Operating Mode field)
> >
> > For Operating Mode Notification, an RX NSS change to a value greater
> > than AP's maximum NSS should not be allowed.
> > Hence allow NSS change only up to maximum NSS that is negotiated and
> > capped to AP's capability during association.
>
> That seems reasonable. Might be worth noting in a comment in the code
> which AP has such bugs though, just so we know it later.
>
These are found during fuzz testing by forcefully sending VHT Op. mode notif. frames from STA with
Random rx_nss values. We accepted all the values.
> > if (link_sta->pub->rx_nss != nss) {
> > - link_sta->pub->rx_nss = nss;
> > - sta_opmode.rx_nss = nss;
> > - changed |= IEEE80211_RC_NSS_CHANGED;
> > - sta_opmode.changed |= STA_OPMODE_N_SS_CHANGED;
> > + cur_nss = link_sta->pub->rx_nss;
> > + link_sta->pub->rx_nss = 0;
> > + ieee80211_sta_set_rx_nss(link_sta);
> > + /* Do not allow an nss change to rx_nss greater than
> max_nss
> > + * negotiated and capped to APs capability during
> association.
> > + */
> > + if (nss <= link_sta->pub->rx_nss) {
> > + link_sta->pub->rx_nss = nss;
>
> That, however, doesn't seem right. It means that you can only ever reduce
> the RX NSS, not switch it around within the originally negotiated range.
>
Not sure if I understood you comment.
We reset Sta's rx_nss and make a call to ieee80211_sta_set_rx_nss() which will set the same to max nss negotiated, so this
Will allow any nss change within negotiated range, isn’t it ?
> johannes
On Tue, 2022-12-27 at 08:04 +0000, Rameshkumar Sundaram (QUIC) wrote:
> >
> > > + if (nss <= link_sta->pub->rx_nss) {
> > > + link_sta->pub->rx_nss = nss;
> >
> > That, however, doesn't seem right. It means that you can only ever
> > reduce
> > the RX NSS, not switch it around within the originally negotiated
> > range.
> >
> Not sure if I understood you comment.
> We reset Sta's rx_nss
I don't see where it's being reset?
The way I'm reading this, you check nss<=rx_nss and then set rx_nss=nss.
I didn't see any code that sets rx_nss higher, but maybe I missed it?
So if say rx_nss is 4, and nss is 2, then we set rx_nss to 2. But now if
the AP wants to switch back to 4, nss will be 4, rx_nss will be 2, and
the change is ignored, no?
johannes
> -----Original Message-----
> From: Johannes Berg <[email protected]>
> Sent: Tuesday, December 27, 2022 9:13 PM
> To: Rameshkumar Sundaram (QUIC) <[email protected]>
> Cc: [email protected]
> Subject: Re: [PATCH] wifi: mac80211: Allow NSS change only up to capability
>
> On Tue, 2022-12-27 at 08:04 +0000, Rameshkumar Sundaram (QUIC) wrote:
> > >
> > > > + if (nss <= link_sta->pub->rx_nss) {
> > > > + link_sta->pub->rx_nss = nss;
> > >
> > > That, however, doesn't seem right. It means that you can only ever
> > > reduce the RX NSS, not switch it around within the originally
> > > negotiated range.
> > >
> > Not sure if I understood you comment.
> > We reset Sta's rx_nss
>
> I don't see where it's being reset?
>
> The way I'm reading this, you check nss<=rx_nss and then set rx_nss=nss.
> I didn't see any code that sets rx_nss higher, but maybe I missed it?
>
+ cur_nss = link_sta->pub->rx_nss;
+ link_sta->pub->rx_nss = 0;
+ ieee80211_sta_set_rx_nss(link_sta);
Here we take copy of current rx_nss of STA (that sent the VHT Op. mode notif frame to AP), reset it and call
ieee80211_sta_set_rx_nss() which will set link_sta->pub->rx_nss to Max nss that was capped and
stored for this STA during association.
Hence Below check will allow an nss change until max_nss at any point.
+ if (nss <= link_sta->pub->rx_nss) {
+ link_sta->pub->rx_nss = nss;
> So if say rx_nss is 4, and nss is 2, then we set rx_nss to 2. But now if the AP
> wants to switch back to 4, nss will be 4, rx_nss will be 2, and the change is
> ignored, no?
>
> johannes