wifi: mac80211: Allow NSS change only up to capability

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 <quic_ramess@quicinc.com>
---
 net/mac80211/vht.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

> -----Original Message-----
> From: Johannes Berg <johannes@sipsolutions.net>
> Sent: Friday, October 21, 2022 5:13 PM
> To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com>
> Cc: linux-wireless@vger.kernel.org
> 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 <johannes@sipsolutions.net>
> Sent: Tuesday, December 27, 2022 9:13 PM
> To: Rameshkumar Sundaram (QUIC) <quic_ramess@quicinc.com>
> Cc: linux-wireless@vger.kernel.org
> 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