| Message ID | 20240726031520.7616-1-pkshih@realtek.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mac80211: avoid warning of no supported legacy rate if empty rate mask for rate control | expand |
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
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
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 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(-)