LinuxLists.cc - [PATCH] wifi: mac80211: don't use rate mask for scanning

2024-03-26 21:10:27

Subject: [PATCH] wifi: mac80211: don't use rate mask for scanning

From: Johannes Berg <[email protected]>

The rate mask is intended for use during operation, and
can be set to only have masks for the currently active
band. As such, it cannot be used for scanning which can
be on other bands as well.

Simply ignore the rate masks during scanning to avoid
warnings from incorrect settings.

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=fdc5123366fb9c3fdc6d
Co-developed-by: Dmitry Antipov <[email protected]>
Signed-off-by: Dmitry Antipov <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
include/net/mac80211.h | 3 +++
net/mac80211/rate.c | 6 +++++-
net/mac80211/scan.c | 1 +
net/mac80211/tx.c | 13 +++++++++----
4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 353488ab94a2..2d7f87bc5324 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -953,6 +953,8 @@ enum mac80211_tx_info_flags {
* of their QoS TID or other priority field values.
* @IEEE80211_TX_CTRL_MCAST_MLO_FIRST_TX: first MLO TX, used mostly internally
* for sequence number assignment
+ * @IEEE80211_TX_CTRL_SCAN_TX: Indicates that this frame is transmitted
+ * due to scanning, not in normal operation on the interface.
* @IEEE80211_TX_CTRL_MLO_LINK: If not @IEEE80211_LINK_UNSPECIFIED, this
* frame should be transmitted on the specific link. This really is
* only relevant for frames that do not have data present, and is
@@ -973,6 +975,7 @@ enum mac80211_tx_control_flags {
IEEE80211_TX_CTRL_NO_SEQNO = BIT(7),
IEEE80211_TX_CTRL_DONT_REORDER = BIT(8),
IEEE80211_TX_CTRL_MCAST_MLO_FIRST_TX = BIT(9),
+ IEEE80211_TX_CTRL_SCAN_TX = BIT(10),
IEEE80211_TX_CTRL_MLO_LINK = 0xf0000000,
};

diff --git a/net/mac80211/rate.c b/net/mac80211/rate.c
index 23404b275457..4dc1def69548 100644
--- a/net/mac80211/rate.c
+++ b/net/mac80211/rate.c
@@ -877,6 +877,7 @@ void ieee80211_get_tx_rates(struct ieee80211_vif *vif,
struct ieee80211_sub_if_data *sdata;
struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
struct ieee80211_supported_band *sband;
+ u32 mask = ~0;

rate_control_fill_sta_table(sta, info, dest, max_rates);

@@ -889,9 +890,12 @@ void ieee80211_get_tx_rates(struct ieee80211_vif *vif,
if (ieee80211_is_tx_data(skb))
rate_control_apply_mask(sdata, sta, sband, dest, max_rates);

+ if (!(info->control.flags & IEEE80211_TX_CTRL_SCAN_TX))
+ mask = sdata->rc_rateidx_mask[info->band];
+
if (dest[0].idx < 0)
__rate_control_send_low(&sdata->local->hw, sband, sta, info,
- sdata->rc_rateidx_mask[info->band]);
+ mask);

if (sta)
rate_fixup_ratelist(vif, sband, info, dest, max_rates);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 0429e59ba387..73850312580f 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -648,6 +648,7 @@ static void ieee80211_send_scan_probe_req(struct ieee80211_sub_if_data *sdata,
cpu_to_le16(IEEE80211_SN_TO_SEQ(sn));
}
IEEE80211_SKB_CB(skb)->flags |= tx_flags;
+ IEEE80211_SKB_CB(skb)->control.flags |= IEEE80211_TX_CTRL_SCAN_TX;
ieee80211_tx_skb_tid_band(sdata, skb, 7, channel->band);
}
}
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 6bf223e6cd1a..cfd0a62d0152 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -698,11 +698,16 @@ ieee80211_tx_h_rate_ctrl(struct ieee80211_tx_data *tx)
txrc.bss_conf = &tx->sdata->vif.bss_conf;
txrc.skb = tx->skb;
txrc.reported_rate.idx = -1;
- txrc.rate_idx_mask = tx->sdata->rc_rateidx_mask[info->band];

- if (tx->sdata->rc_has_mcs_mask[info->band])
- txrc.rate_idx_mcs_mask =
- tx->sdata->rc_rateidx_mcs_mask[info->band];
+ if (unlikely(info->control.flags & IEEE80211_TX_CTRL_SCAN_TX)) {
+ txrc.rate_idx_mask = ~0;
+ } else {
+ txrc.rate_idx_mask = tx->sdata->rc_rateidx_mask[info->band];
+
+ if (tx->sdata->rc_has_mcs_mask[info->band])
+ txrc.rate_idx_mcs_mask =
+ tx->sdata->rc_rateidx_mcs_mask[info->band];
+ }

txrc.bss = (tx->sdata->vif.type == NL80211_IFTYPE_AP ||
tx->sdata->vif.type == NL80211_IFTYPE_MESH_POINT ||
--
2.44.0

2024-03-27 06:57:07

by Dmitry Antipov

[permalink] [raw]

Subject: Re: [PATCH] wifi: mac80211: don't use rate mask for scanning

2024-03-29 09:53:20

by Dmitry Antipov

[permalink] [raw]

Subject: Re: [PATCH] wifi: mac80211: don't use rate mask for scanning

On 3/27/24 00:08, Johannes Berg wrote:

> From: Johannes Berg <[email protected]>
>
> The rate mask is intended for use during operation, and
> can be set to only have masks for the currently active
> band. As such, it cannot be used for scanning which can
> be on other bands as well.
>
> Simply ignore the rate masks during scanning to avoid
> warnings from incorrect settings.
>
> Reported-by: [email protected]
> Closes: https://syzkaller.appspot.com/bug?extid=fdc5123366fb9c3fdc6d
> Co-developed-by: Dmitry Antipov <[email protected]>
> Signed-off-by: Dmitry Antipov <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>

Ugh. Fedor has reported (and I have confirmed) that this still may be
reproduced with https://syzkaller.appspot.com/text?tag=ReproC&x=12a8fd7f680000
as:

[ 40.293787][ T5149] no supported rates for sta 08:02:11:00:00:01 (0xf, band 0) in rate_mask 0xfff with flags 0x10
[ 40.294789][ T5149] WARNING: CPU: 1 PID: 5149 at net/mac80211/rate.c:380 __rate_control_send_low+0x6af/0x810
[ 40.295624][ T5149] Modules linked in:
[ 40.296369][ T5149] CPU: 1 PID: 5149 Comm: repro3 Not tainted 6.9.0-rc1-00179-g46ad21a6b2e3 #1
[ 40.296918][ T5149] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
[ 40.297534][ T5149] RIP: 0010:__rate_control_send_low+0x6af/0x810
[ 40.297946][ T5149] Code: 8b ac a8 d4 00 00 00 e8 df 4d 4f f7 44 8b 44 24 04 45 89 f9 89 d9 48 8b 74 24 18 89 ea 48 c7 c7 60 68 4e 8c e8 62 a0 11 f7 90 <0f> 0b 90 90 e9 1f fd ff ff 48 8b 7c 24 28
e8 ce 16 ab f7 e9 13 fc
[ 40.299218][ T5149] RSP: 0018:ffffc9000350ed40 EFLAGS: 00010282
[ 40.299624][ T5149] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8150f9b9
[ 40.300192][ T5149] RDX: ffff88810b509cc0 RSI: ffffffff8150f9c6 RDI: 0000000000000001
[ 40.300743][ T5149] RBP: 000000000000000f R08: 0000000000000001 R09: 0000000000000000
[ 40.301291][ T5149] R10: 0000000000000000 R11: 0000000000000006 R12: ffff88801985f228
[ 40.301812][ T5149] R13: ffff888107edb088 R14: 000000000000000c R15: 0000000000000010
[ 40.302335][ T5149] FS: 00007f16474fe740(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
[ 40.302945][ T5149] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 40.303385][ T5149] CR2: 00007f16474ff0e8 CR3: 0000000109dc0000 CR4: 00000000000006f0
[ 40.303957][ T5149] Call Trace:
[ 40.304221][ T5149] <TASK>
[ 40.308220][ T5149] rate_control_send_low+0x116/0x7e0
[ 40.308786][ T5149] rate_control_get_rate+0x1be/0x590
[ 40.309153][ T5149] ieee80211_tx_h_rate_ctrl+0xaa1/0x1a50
[ 40.310581][ T5149] invoke_tx_handlers_late+0x133b/0x2ae0
[ 40.312476][ T5149] ieee80211_tx+0x306/0x420
[ 40.314290][ T5149] ieee80211_xmit+0x30e/0x3e0
[ 40.314651][ T5149] __ieee80211_tx_skb_tid_band+0x29b/0x700
[ 40.315090][ T5149] ieee80211_tx_skb_tid+0x176/0x4f0
[ 40.315483][ T5149] ieee80211_mgmt_tx+0x129a/0x2160
[ 40.315868][ T5149] cfg80211_mlme_mgmt_tx+0x910/0x1570
[ 40.316277][ T5149] nl80211_tx_mgmt+0x7ad/0xcf0
[ 40.317822][ T5149] genl_family_rcv_msg_doit+0x205/0x2f0
[ 40.319083][ T5149] genl_rcv_msg+0x56c/0x810
[ 40.321628][ T5149] netlink_rcv_skb+0x16e/0x440
[ 40.324076][ T5149] genl_rcv+0x28/0x40
[ 40.324359][ T5149] netlink_unicast+0x545/0x820
[ 40.325810][ T5149] netlink_sendmsg+0x8b8/0xd70
[ 40.327175][ T5149] ____sys_sendmsg+0xacf/0xca0
[ 40.328673][ T5149] ___sys_sendmsg+0x135/0x1e0
[ 40.330261][ T5149] __sys_sendmsg+0x117/0x1f0
[ 40.330761][ T5149] do_syscall_64+0xd3/0x260
[ 40.331047][ T5149] entry_SYSCALL_64_after_hwframe+0x6d/0x75

Note that the backtrace is different and this
one comes from MLME rather than scanning.

Dmitry

2024-03-29 10:51:58

by Johannes Berg

[permalink] [raw]

Subject: Re: [PATCH] wifi: mac80211: don't use rate mask for scanning

On Fri, 2024-03-29 at 12:47 +0300, Dmitry Antipov wrote:
> On 3/27/24 00:08, Johannes Berg wrote:
>
> > From: Johannes Berg <[email protected]>
> >
> > The rate mask is intended for use during operation, and
> > can be set to only have masks for the currently active
> > band. As such, it cannot be used for scanning which can
> > be on other bands as well.
> >
> > Simply ignore the rate masks during scanning to avoid
> > warnings from incorrect settings.
> >
> > Reported-by: [email protected]
> > Closes: https://syzkaller.appspot.com/bug?extid=fdc5123366fb9c3fdc6d
> > Co-developed-by: Dmitry Antipov <[email protected]>
> > Signed-off-by: Dmitry Antipov <[email protected]>
> > Signed-off-by: Johannes Berg <[email protected]>
>
> Ugh. Fedor has reported (and I have confirmed) that this still may be
> reproduced with https://syzkaller.appspot.com/text?tag=ReproC&x=12a8fd7f680000
> as:
>
> [ 40.293787][ T5149] no supported rates for sta 08:02:11:00:00:01 (0xf, band 0) in rate_mask 0xfff with flags 0x10
> [ 40.294789][ T5149] WARNING: CPU: 1 PID: 5149 at net/mac80211/rate.c:380 __rate_control_send_low+0x6af/0x810
> [ 40.295624][ T5149] Modules linked in:
> [ 40.296369][ T5149] CPU: 1 PID: 5149 Comm: repro3 Not tainted 6.9.0-rc1-00179-g46ad21a6b2e3 #1
> [ 40.296918][ T5149] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
> [ 40.297534][ T5149] RIP: 0010:__rate_control_send_low+0x6af/0x810
> [ 40.297946][ T5149] Code: 8b ac a8 d4 00 00 00 e8 df 4d 4f f7 44 8b 44 24 04 45 89 f9 89 d9 48 8b 74 24 18 89 ea 48 c7 c7 60 68 4e 8c e8 62 a0 11 f7 90 <0f> 0b 90 90 e9 1f fd ff ff 48 8b 7c 24 28
> e8 ce 16 ab f7 e9 13 fc
> [ 40.299218][ T5149] RSP: 0018:ffffc9000350ed40 EFLAGS: 00010282
> [ 40.299624][ T5149] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8150f9b9
> [ 40.300192][ T5149] RDX: ffff88810b509cc0 RSI: ffffffff8150f9c6 RDI: 0000000000000001
> [ 40.300743][ T5149] RBP: 000000000000000f R08: 0000000000000001 R09: 0000000000000000
> [ 40.301291][ T5149] R10: 0000000000000000 R11: 0000000000000006 R12: ffff88801985f228
> [ 40.301812][ T5149] R13: ffff888107edb088 R14: 000000000000000c R15: 0000000000000010
> [ 40.302335][ T5149] FS: 00007f16474fe740(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> [ 40.302945][ T5149] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 40.303385][ T5149] CR2: 00007f16474ff0e8 CR3: 0000000109dc0000 CR4: 00000000000006f0
> [ 40.303957][ T5149] Call Trace:
> [ 40.304221][ T5149] <TASK>
> [ 40.308220][ T5149] rate_control_send_low+0x116/0x7e0
> [ 40.308786][ T5149] rate_control_get_rate+0x1be/0x590
> [ 40.309153][ T5149] ieee80211_tx_h_rate_ctrl+0xaa1/0x1a50
> [ 40.310581][ T5149] invoke_tx_handlers_late+0x133b/0x2ae0
> [ 40.312476][ T5149] ieee80211_tx+0x306/0x420
> [ 40.314290][ T5149] ieee80211_xmit+0x30e/0x3e0
> [ 40.314651][ T5149] __ieee80211_tx_skb_tid_band+0x29b/0x700
> [ 40.315090][ T5149] ieee80211_tx_skb_tid+0x176/0x4f0
> [ 40.315483][ T5149] ieee80211_mgmt_tx+0x129a/0x2160
> [ 40.315868][ T5149] cfg80211_mlme_mgmt_tx+0x910/0x1570
> [ 40.316277][ T5149] nl80211_tx_mgmt+0x7ad/0xcf0
> [ 40.317822][ T5149] genl_family_rcv_msg_doit+0x205/0x2f0
> [ 40.319083][ T5149] genl_rcv_msg+0x56c/0x810
> [ 40.321628][ T5149] netlink_rcv_skb+0x16e/0x440
> [ 40.324076][ T5149] genl_rcv+0x28/0x40
> [ 40.324359][ T5149] netlink_unicast+0x545/0x820
> [ 40.325810][ T5149] netlink_sendmsg+0x8b8/0xd70
> [ 40.327175][ T5149] ____sys_sendmsg+0xacf/0xca0
> [ 40.328673][ T5149] ___sys_sendmsg+0x135/0x1e0
> [ 40.330261][ T5149] __sys_sendmsg+0x117/0x1f0
> [ 40.330761][ T5149] do_syscall_64+0xd3/0x260
> [ 40.331047][ T5149] entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> Note that the backtrace is different and this
> one comes from MLME rather than scanning.
>

Yeah, that's not a huge surprise. I'll have to check where this comes
from, but chances are also there we shouldn't use the rate mask for all
the same reasons outlined previously for scanning.

johannes

2024-03-29 10:54:07

by Fedor Pchelkin

[permalink] [raw]

Subject: Re: [PATCH] wifi: mac80211: don't use rate mask for scanning

On Fri, 29. Mar 12:47, Dmitry Antipov wrote:
> On 3/27/24 00:08, Johannes Berg wrote:
>
> > From: Johannes Berg <[email protected]>
> >
> > The rate mask is intended for use during operation, and
> > can be set to only have masks for the currently active
> > band. As such, it cannot be used for scanning which can
> > be on other bands as well.
> >
> > Simply ignore the rate masks during scanning to avoid
> > warnings from incorrect settings.
> >
> > Reported-by: [email protected]
> > Closes: https://syzkaller.appspot.com/bug?extid=fdc5123366fb9c3fdc6d
> > Co-developed-by: Dmitry Antipov <[email protected]>
> > Signed-off-by: Dmitry Antipov <[email protected]>
> > Signed-off-by: Johannes Berg <[email protected]>
>
> Ugh. Fedor has reported (and I have confirmed) that this still may be
> reproduced with https://syzkaller.appspot.com/text?tag=ReproC&x=12a8fd7f680000
> as:
>
> [ 40.293787][ T5149] no supported rates for sta 08:02:11:00:00:01 (0xf, band 0) in rate_mask 0xfff with flags 0x10
> [ 40.294789][ T5149] WARNING: CPU: 1 PID: 5149 at net/mac80211/rate.c:380 __rate_control_send_low+0x6af/0x810
> [ 40.295624][ T5149] Modules linked in:
> [ 40.296369][ T5149] CPU: 1 PID: 5149 Comm: repro3 Not tainted 6.9.0-rc1-00179-g46ad21a6b2e3 #1
> [ 40.296918][ T5149] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
> [ 40.297534][ T5149] RIP: 0010:__rate_control_send_low+0x6af/0x810
> [ 40.297946][ T5149] Code: 8b ac a8 d4 00 00 00 e8 df 4d 4f f7 44 8b 44 24
> 04 45 89 f9 89 d9 48 8b 74 24 18 89 ea 48 c7 c7 60 68 4e 8c e8 62 a0 11 f7
> 90 <0f> 0b 90 90 e9 1f fd ff ff 48 8b 7c 24 28 e8 ce 16 ab f7 e9 13 fc
> [ 40.299218][ T5149] RSP: 0018:ffffc9000350ed40 EFLAGS: 00010282
> [ 40.299624][ T5149] RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff8150f9b9
> [ 40.300192][ T5149] RDX: ffff88810b509cc0 RSI: ffffffff8150f9c6 RDI: 0000000000000001
> [ 40.300743][ T5149] RBP: 000000000000000f R08: 0000000000000001 R09: 0000000000000000
> [ 40.301291][ T5149] R10: 0000000000000000 R11: 0000000000000006 R12: ffff88801985f228
> [ 40.301812][ T5149] R13: ffff888107edb088 R14: 000000000000000c R15: 0000000000000010
> [ 40.302335][ T5149] FS: 00007f16474fe740(0000) GS:ffff888135c00000(0000) knlGS:0000000000000000
> [ 40.302945][ T5149] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 40.303385][ T5149] CR2: 00007f16474ff0e8 CR3: 0000000109dc0000 CR4: 00000000000006f0
> [ 40.303957][ T5149] Call Trace:
> [ 40.304221][ T5149] <TASK>
> [ 40.308220][ T5149] rate_control_send_low+0x116/0x7e0
> [ 40.308786][ T5149] rate_control_get_rate+0x1be/0x590
> [ 40.309153][ T5149] ieee80211_tx_h_rate_ctrl+0xaa1/0x1a50
> [ 40.310581][ T5149] invoke_tx_handlers_late+0x133b/0x2ae0
> [ 40.312476][ T5149] ieee80211_tx+0x306/0x420
> [ 40.314290][ T5149] ieee80211_xmit+0x30e/0x3e0
> [ 40.314651][ T5149] __ieee80211_tx_skb_tid_band+0x29b/0x700
> [ 40.315090][ T5149] ieee80211_tx_skb_tid+0x176/0x4f0
> [ 40.315483][ T5149] ieee80211_mgmt_tx+0x129a/0x2160
> [ 40.315868][ T5149] cfg80211_mlme_mgmt_tx+0x910/0x1570
> [ 40.316277][ T5149] nl80211_tx_mgmt+0x7ad/0xcf0
> [ 40.317822][ T5149] genl_family_rcv_msg_doit+0x205/0x2f0
> [ 40.319083][ T5149] genl_rcv_msg+0x56c/0x810
> [ 40.321628][ T5149] netlink_rcv_skb+0x16e/0x440
> [ 40.324076][ T5149] genl_rcv+0x28/0x40
> [ 40.324359][ T5149] netlink_unicast+0x545/0x820
> [ 40.325810][ T5149] netlink_sendmsg+0x8b8/0xd70
> [ 40.327175][ T5149] ____sys_sendmsg+0xacf/0xca0
> [ 40.328673][ T5149] ___sys_sendmsg+0x135/0x1e0
> [ 40.330261][ T5149] __sys_sendmsg+0x117/0x1f0
> [ 40.330761][ T5149] do_syscall_64+0xd3/0x260
> [ 40.331047][ T5149] entry_SYSCALL_64_after_hwframe+0x6d/0x75
>
> Note that the backtrace is different and this
> one comes from MLME rather than scanning.
>
> Dmitry
>

Yeah, I think it might be caused by a completely different scenario not
related to scanning - which can be seen from the backtrace. So it may need
a different analysis and probably a fix in another place.

The warnings while scanning have been fixed with the proposed patch, I can
confirm, too.

--
Fedor