[PATCH] wifi: mac80211: avoid warning of no supported legacy rate if empty rate mask for rate control

[PATCH] wifi: mac80211: avoid warning of no supported legacy rate if empty rate mask for rate control

[Ping-Ke Shih]

The commit 9df66d5b9f45 ("cfg80211: fix default HE tx bitrate mask in 2G
band") correct bitmask of HE MCS, and settings of empty legacy rate plus
HE MCS rate are correctly recognized instead of returning -EINVAL,
so empty legacy rate propagates to __rate_control_send_low() and warn
no supported rate.

Since the rate_mask is intentionally set to empty via nl80211, change logic
to avoid warning no supported rate if rate_mask is empty.

Reported-by: syzbot+8dd98a9e98ee28dc484a@syzkaller.appspotmail.com
Fixes: 9df66d5b9f45 ("cfg80211: fix default HE tx bitrate mask in 2G band")
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 net/mac80211/rate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 4dc1def69548..5787cb20de42 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -377,7 +377,7 @@ static void __rate_control_send_low(struct ieee80211_hw *hw,
 		info->control.rates[0].idx = i;
 		break;
 	}
-	WARN_ONCE(i == sband->n_bitrates,
+	WARN_ONCE(i == sband->n_bitrates && rate_mask,
 		  "no supported rates for sta %pM (0x%x, band %d) in rate_mask 0x%x with flags 0x%x\n",
 		  sta ? sta->addr : NULL,
 		  sta ? sta->deflink.supp_rates[sband->band] : -1,
-- 
2.25.1

Re: [PATCH] wifi: mac80211: avoid warning of no supported legacy rate if empty rate mask for rate control

[Johannes Berg]

Hi PK,

Thanks for taking a lot at the syzbot report! It's been on my list for a
while, but didn't get to it.

> The commit 9df66d5b9f45 ("cfg80211: fix default HE tx bitrate mask in 2G
> band") correct bitmask of HE MCS, and settings of empty legacy rate plus
> HE MCS rate are correctly recognized instead of returning -EINVAL,
> so empty legacy rate propagates to __rate_control_send_low() and warn
> no supported rate.
> 
> Since the rate_mask is intentionally set to empty via nl80211, 

That's all true, however,

> change logic
> to avoid warning no supported rate if rate_mask is empty.

I don't necessarily think this follows.

> diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
> index 4dc1def69548..5787cb20de42 100644
> --- a/net/mac80211/rate.c
> +++ b/net/mac80211/rate.c
> @@ -377,7 +377,7 @@ static void __rate_control_send_low(struct ieee80211_hw *hw,
>  		info->control.rates[0].idx = i;
>  		break;
>  	}
> -	WARN_ONCE(i == sband->n_bitrates,
> +	WARN_ONCE(i == sband->n_bitrates && rate_mask,
>  		  "no supported rates for sta %pM (0x%x, band %d) in rate_mask 0x%x with flags 0x%x\n",
>  		  sta ? sta->addr : NULL,
>  		  sta ? sta->deflink.supp_rates[sband->band] : -1,

The warning is still valid - we're trying to pick a low rate with a NULL
station (i.e. we don't even really know where to send the frame), but we
don't have any rates to do so with.

Obviously this will remove the warning in this case, but I think the
underlying issue is that we're actually using the rate mask, intended
for the connection on the interface, for offchannel TX (looking at the
stack dump).

We had this precise discussion previously for scanning, and just like
there, fixed in ab9177d83c04 ("wifi: mac80211: don't use rate mask for
scanning"), I feel the right way to approach this issue here would be to
similarly not use the rate mask for offchannel TX, which is I think
pretty much the same situation, you could have a rate mask set for only
2.4 GHz where the connection is (and empty for other bands), which is
accepted by cfg80211 and mac80211, but then do offchannel TX on 5 GHz
anyway.

So I think the right way to approach this would be to do something like 

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 28d03196ef75..33361b4d9acf 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -830,6 +830,8 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 		flags = IEEE80211_TX_INTFL_NL80211_FRAME_TX |
 			IEEE80211_TX_CTL_REQ_TX_STATUS;
 
+	flags |= IEEE80211_TX_CTRL_SCAN_TX;
+
 	if (params->no_cck)
 		flags |= IEEE80211_TX_CTL_NO_CCK_RATE;
 

though at that point we need to rename that flag too, I guess.


johannes

RE: [PATCH] wifi: mac80211: avoid warning of no supported legacy rate if empty rate mask for rate control

[Ping-Ke Shih]

Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> We had this precise discussion previously for scanning, and just like
> there, fixed in ab9177d83c04 ("wifi: mac80211: don't use rate mask for
> scanning"), I feel the right way to approach this issue here would be to
> similarly not use the rate mask for offchannel TX, which is I think
> pretty much the same situation, you could have a rate mask set for only
> 2.4 GHz where the connection is (and empty for other bands), which is
> accepted by cfg80211 and mac80211, but then do offchannel TX on 5 GHz
> anyway.
> 
> So I think the right way to approach this would be to do something like
> 
> diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
> index 28d03196ef75..33361b4d9acf 100644
> --- a/net/mac80211/offchannel.c
> +++ b/net/mac80211/offchannel.c
> @@ -830,6 +830,8 @@ int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>                 flags = IEEE80211_TX_INTFL_NL80211_FRAME_TX |
>                         IEEE80211_TX_CTL_REQ_TX_STATUS;
> 
> +       flags |= IEEE80211_TX_CTRL_SCAN_TX;
> +
>         if (params->no_cck)
>                 flags |= IEEE80211_TX_CTL_NO_CCK_RATE;
> 
> 
> though at that point we need to rename that flag too, I guess.

Thanks for the suggestions. I made and sent a patch [1] that works well in
my side using syzbot's reproducer code. 

[1] https://lore.kernel.org/linux-wireless/20240729074816.20323-1-pkshih@realtek.com/T/#u