* Re: [syzbot] [wireless?] WARNING in rate_control_rate_init (2)
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
@ 2023-11-28 23:57 ` syzbot
2023-11-29 3:06 ` [syzbot] [wireless?] WARNING in rate_control_rate_init syzbot
` (4 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-28 23:57 UTC (permalink / raw)
To: davem, edumazet, johannes.berg, johannes, kuba, linux-kernel,
linux-wireless, llvm, nathan, ndesaulniers, netdev, pabeni,
syzkaller-bugs, trix
syzbot has bisected this issue to:
commit b303835dabe0340f932ebb4e260d2229f79b0684
Author: Johannes Berg <johannes.berg@intel.com>
Date: Sat Jul 23 20:08:49 2022 +0000
wifi: mac80211: accept STA changes without link changes
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=125a86dce80000
start commit: a214724554ae Merge tag 'wireless-next-2023-11-27' of git:/..
git tree: net-next
final oops: https://syzkaller.appspot.com/x/report.txt?x=115a86dce80000
console output: https://syzkaller.appspot.com/x/log.txt?x=165a86dce80000
kernel config: https://syzkaller.appspot.com/x/.config?x=abf6d5a82dab01fe
dashboard link: https://syzkaller.appspot.com/bug?extid=62d7eef57b09bfebcd84
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10a4fc64e80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1363b22ce80000
Reported-by: syzbot+62d7eef57b09bfebcd84@syzkaller.appspotmail.com
Fixes: b303835dabe0 ("wifi: mac80211: accept STA changes without link changes")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [syzbot] [wireless?] WARNING in rate_control_rate_init
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
2023-11-28 23:57 ` syzbot
@ 2023-11-29 3:06 ` syzbot
2023-11-29 4:04 ` syzbot
` (3 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-29 3:06 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: [wireless?] WARNING in rate_control_rate_init
Author: eadavis@qq.com
please test WARNING in rate_control_rate_init
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2332e0ab53
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..13d52452a124 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1796,7 +1796,7 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
!params->supported_rates_len &&
!params->ht_capa && !params->vht_capa &&
!params->he_capa && !params->eht_capa &&
- !params->opmode_notif_used)
+ !params->opmode_notif_used && 0)
return 0;
if (!link || !link_sta)
@@ -1817,6 +1817,7 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
} else if (new_link) {
return -EINVAL;
}
+ printk("b, %p \n", rcu_access_pointer(sdata->vif.bss_conf.chanctx_conf));
if (params->txpwr_set) {
link_sta->pub->txpwr.type = params->txpwr.type;
@@ -1868,6 +1869,7 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
params->opmode_notif,
sband->band);
}
+ printk("e, %p \n", rcu_access_pointer(sdata->vif.bss_conf.chanctx_conf));
return ret;
}
@@ -1982,6 +1984,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->listen_interval >= 0)
sta->listen_interval = params->listen_interval;
+ printk("b, stp: %d, sa: %d, src: %d\n",
+ test_sta_flag(sta, WLAN_STA_TDLS_PEER),
+ test_sta_flag(sta, WLAN_STA_ASSOC),
+ test_sta_flag(sta, WLAN_STA_RATE_CONTROL));
ret = sta_link_apply_parameters(local, sta, false,
¶ms->link_sta_params);
if (ret)
@@ -1996,6 +2002,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->airtime_weight)
sta->airtime_weight = params->airtime_weight;
+ printk("a, stp: %d, sa: %d, src: %d\n",
+ test_sta_flag(sta, WLAN_STA_TDLS_PEER),
+ test_sta_flag(sta, WLAN_STA_ASSOC),
+ test_sta_flag(sta, WLAN_STA_RATE_CONTROL));
/* set the STA state after all sta info from usermode has been set */
if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) ||
set & BIT(NL80211_STA_FLAG_ASSOCIATED)) {
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [syzbot] [wireless?] WARNING in rate_control_rate_init
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
2023-11-28 23:57 ` syzbot
2023-11-29 3:06 ` [syzbot] [wireless?] WARNING in rate_control_rate_init syzbot
@ 2023-11-29 4:04 ` syzbot
2023-11-29 5:48 ` [PATCH] wifi: mac80211: sband's null check should precede params Edward Adam Davis
` (2 subsequent siblings)
5 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-29 4:04 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: [wireless?] WARNING in rate_control_rate_init
Author: eadavis@qq.com
please test WARNING in rate_control_rate_init
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2332e0ab53
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..e97ed85b7723 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1787,22 +1787,12 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
rcu_dereference_protected(sta->link[link_id],
lockdep_is_held(&local->hw.wiphy->mtx));
- /*
- * If there are no changes, then accept a link that doesn't exist,
- * unless it's a new link.
- */
- if (params->link_id < 0 && !new_link &&
- !params->link_mac && !params->txpwr_set &&
- !params->supported_rates_len &&
- !params->ht_capa && !params->vht_capa &&
- !params->he_capa && !params->eht_capa &&
- !params->opmode_notif_used)
- return 0;
-
+ printk("%p, %p, %d\n", link, link_sta, new_link);
if (!link || !link_sta)
return -EINVAL;
sband = ieee80211_get_link_sband(link);
+ printk("%p\n", sband);
if (!sband)
return -EINVAL;
@@ -1812,11 +1802,23 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
memcpy(link_sta->pub->addr, params->link_mac, ETH_ALEN);
} else if (!ether_addr_equal(link_sta->addr,
params->link_mac)) {
+ printk("%s\n", __func__);
return -EINVAL;
}
} else if (new_link) {
return -EINVAL;
}
+ /*
+ * If there are no changes, then accept a link that doesn't exist,
+ * unless it's a new link.
+ */
+ if (params->link_id < 0 && !new_link &&
+ !params->link_mac && !params->txpwr_set &&
+ !params->supported_rates_len &&
+ !params->ht_capa && !params->vht_capa &&
+ !params->he_capa && !params->eht_capa &&
+ !params->opmode_notif_used)
+ return 0;
if (params->txpwr_set) {
link_sta->pub->txpwr.type = params->txpwr.type;
@@ -1982,6 +1985,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->listen_interval >= 0)
sta->listen_interval = params->listen_interval;
+ printk("b, stp: %d, sa: %d, src: %d\n",
+ test_sta_flag(sta, WLAN_STA_TDLS_PEER),
+ test_sta_flag(sta, WLAN_STA_ASSOC),
+ test_sta_flag(sta, WLAN_STA_RATE_CONTROL));
ret = sta_link_apply_parameters(local, sta, false,
¶ms->link_sta_params);
if (ret)
@@ -1996,6 +2003,10 @@ static int sta_apply_parameters(struct ieee80211_local *local,
if (params->airtime_weight)
sta->airtime_weight = params->airtime_weight;
+ printk("a, stp: %d, sa: %d, src: %d\n",
+ test_sta_flag(sta, WLAN_STA_TDLS_PEER),
+ test_sta_flag(sta, WLAN_STA_ASSOC),
+ test_sta_flag(sta, WLAN_STA_RATE_CONTROL));
/* set the STA state after all sta info from usermode has been set */
if (test_sta_flag(sta, WLAN_STA_TDLS_PEER) ||
set & BIT(NL80211_STA_FLAG_ASSOCIATED)) {
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH] wifi: mac80211: sband's null check should precede params
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
` (2 preceding siblings ...)
2023-11-29 4:04 ` syzbot
@ 2023-11-29 5:48 ` Edward Adam Davis
2023-11-29 6:57 ` Johannes Berg
2023-11-29 11:04 ` [syzbot] [wireless?] WARNING in rate_control_rate_init syzbot
2023-11-29 11:26 ` syzbot
5 siblings, 1 reply; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-29 5:48 UTC (permalink / raw)
To: syzbot+62d7eef57b09bfebcd84
Cc: davem, edumazet, johannes, kuba, linux-kernel, linux-wireless,
llvm, nathan, ndesaulniers, netdev, pabeni, syzkaller-bugs, trix
[Syz report]
WARNING: CPU: 1 PID: 5067 at net/mac80211/rate.c:48 rate_control_rate_init+0x540/0x690 net/mac80211/rate.c:48
Modules linked in:
CPU: 1 PID: 5067 Comm: syz-executor413 Not tainted 6.7.0-rc3-syzkaller-00014-gdf60cee26a2e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/10/2023
RIP: 0010:rate_control_rate_init+0x540/0x690 net/mac80211/rate.c:48
Code: 48 c7 c2 00 46 0c 8c be 08 03 00 00 48 c7 c7 c0 45 0c 8c c6 05 70 79 0b 05 01 e8 1b a0 6f f7 e9 e0 fd ff ff e8 61 b3 8f f7 90 <0f> 0b 90 e9 36 ff ff ff e8 53 b3 8f f7 e8 5e 0b 78 f7 31 ff 89 c3
RSP: 0018:ffffc90003c57248 EFLAGS: 00010293
RAX: 0000000000000000 RBX: ffff888016bc4000 RCX: ffffffff89f7d519
RDX: ffff888076d43b80 RSI: ffffffff89f7d6df RDI: 0000000000000005
RBP: ffff88801daaae20 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000002 R12: 0000000000000001
R13: 0000000000000000 R14: ffff888020030e20 R15: ffff888078f08000
FS: 0000555556b94380(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000005fdeb8 CR3: 0000000076d22000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
sta_apply_auth_flags.constprop.0+0x4b7/0x510 net/mac80211/cfg.c:1674
sta_apply_parameters+0xaf1/0x16c0 net/mac80211/cfg.c:2002
ieee80211_add_station+0x3fa/0x6c0 net/mac80211/cfg.c:2068
rdev_add_station net/wireless/rdev-ops.h:201 [inline]
nl80211_new_station+0x13ba/0x1a70 net/wireless/nl80211.c:7603
genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2545
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1368
netlink_sendmsg+0x93c/0xe40 net/netlink/af_netlink.c:1910
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0xd5/0x180 net/socket.c:745
____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
__sys_sendmsg+0x117/0x1e0 net/socket.c:2667
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
[Analysis]
When ieee80211_get_link_sband() fails to find a valid sband and first checks
for params in sta_link_apply_parameters(), it will return 0 due to new_link
being 0, which will lead to an incorrect process after sta_apply_parameters().
[Fix]
First obtain sband and perform a non null check before checking the params.
Fixes: b303835dabe0 ("wifi: mac80211: accept STA changes without link changes")
Reported-and-tested-by: syzbot+62d7eef57b09bfebcd84@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mac80211/cfg.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..8012dcdbcb5f 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1787,6 +1787,13 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
rcu_dereference_protected(sta->link[link_id],
lockdep_is_held(&local->hw.wiphy->mtx));
+ if (!link || !link_sta)
+ return -EINVAL;
+
+ sband = ieee80211_get_link_sband(link);
+ if (!sband)
+ return -EINVAL;
+
/*
* If there are no changes, then accept a link that doesn't exist,
* unless it's a new link.
@@ -1799,13 +1806,6 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
!params->opmode_notif_used)
return 0;
- if (!link || !link_sta)
- return -EINVAL;
-
- sband = ieee80211_get_link_sband(link);
- if (!sband)
- return -EINVAL;
-
if (params->link_mac) {
if (new_link) {
memcpy(link_sta->addr, params->link_mac, ETH_ALEN);
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH] wifi: mac80211: sband's null check should precede params
2023-11-29 5:48 ` [PATCH] wifi: mac80211: sband's null check should precede params Edward Adam Davis
@ 2023-11-29 6:57 ` Johannes Berg
2023-11-29 8:18 ` Edward Adam Davis
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2023-11-29 6:57 UTC (permalink / raw)
To: Edward Adam Davis, syzbot+62d7eef57b09bfebcd84
Cc: davem, edumazet, kuba, linux-kernel, linux-wireless, llvm, nathan,
ndesaulniers, netdev, pabeni, syzkaller-bugs, trix
On Wed, 2023-11-29 at 13:48 +0800, Edward Adam Davis wrote:
>
> [Analysis]
> When ieee80211_get_link_sband() fails to find a valid sband and first checks
> for params in sta_link_apply_parameters(), it will return 0 due to new_link
> being 0, which will lead to an incorrect process after sta_apply_parameters().
>
> [Fix]
> First obtain sband and perform a non null check before checking the params.
Not sure I can even disagree with that analysis, it seems right, but ...
> + if (!link || !link_sta)
> + return -EINVAL;
> +
> + sband = ieee80211_get_link_sband(link);
> + if (!sband)
> + return -EINVAL;
> +
> /*
> * If there are no changes, then accept a link that doesn't exist,
> * unless it's a new link.
There's a comment here which is clearly not true after this change,
since you've already returned for !link_sta?
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] wifi: mac80211: sband's null check should precede params
2023-11-29 6:57 ` Johannes Berg
@ 2023-11-29 8:18 ` Edward Adam Davis
2023-11-29 8:33 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-29 8:18 UTC (permalink / raw)
To: johannes
Cc: davem, eadavis, edumazet, kuba, linux-kernel, linux-wireless,
llvm, nathan, ndesaulniers, netdev, pabeni,
syzbot+62d7eef57b09bfebcd84, syzkaller-bugs, trix
On Wed, 29 Nov 2023 07:57:07 +0100, Johannes Berg wrote:
> > [Analysis]
> > When ieee80211_get_link_sband() fails to find a valid sband and first checks
> > for params in sta_link_apply_parameters(), it will return 0 due to new_link
> > being 0, which will lead to an incorrect process after sta_apply_parameters().
> >
> > [Fix]
> > First obtain sband and perform a non null check before checking the params.
>
> Not sure I can even disagree with that analysis, it seems right, but ...
>
> > + if (!link || !link_sta)
> > + return -EINVAL;
> > +
> > + sband = ieee80211_get_link_sband(link);
> > + if (!sband)
> > + return -EINVAL;
> > +
> > /*
> > * If there are no changes, then accept a link that doesn't exist,
> > * unless it's a new link.
>
> There's a comment here which is clearly not true after this change,
> since you've already returned for !link_sta?
No, after applying my patch, it will return due to !sband.
Edward
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] wifi: mac80211: sband's null check should precede params
2023-11-29 8:18 ` Edward Adam Davis
@ 2023-11-29 8:33 ` Johannes Berg
2023-11-29 8:48 ` Edward Adam Davis
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2023-11-29 8:33 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, kuba, linux-kernel, linux-wireless, llvm, nathan,
ndesaulniers, netdev, pabeni, syzbot+62d7eef57b09bfebcd84,
syzkaller-bugs, trix
On Wed, 2023-11-29 at 16:18 +0800, Edward Adam Davis wrote:
> On Wed, 29 Nov 2023 07:57:07 +0100, Johannes Berg wrote:
> > > [Analysis]
> > > When ieee80211_get_link_sband() fails to find a valid sband and first checks
> > > for params in sta_link_apply_parameters(), it will return 0 due to new_link
> > > being 0, which will lead to an incorrect process after sta_apply_parameters().
> > >
> > > [Fix]
> > > First obtain sband and perform a non null check before checking the params.
> >
> > Not sure I can even disagree with that analysis, it seems right, but ...
> >
> > > + if (!link || !link_sta)
> > > + return -EINVAL;
> > > +
> > > + sband = ieee80211_get_link_sband(link);
> > > + if (!sband)
> > > + return -EINVAL;
> > > +
> > > /*
> > > * If there are no changes, then accept a link that doesn't exist,
> > > * unless it's a new link.
> >
> > There's a comment here which is clearly not true after this change,
> > since you've already returned for !link_sta?
> No, after applying my patch, it will return due to !sband.
>
Right, OK, but the way I read the comment (now) is that it wanted to
accept it in that case?
That said, I just threw the patch into our internal testing machinery
quickly (probably has more MLO tests than upstream hostap for now), and
it worked just fine ...
Maybe we should just remove the comment?
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] wifi: mac80211: sband's null check should precede params
2023-11-29 8:33 ` Johannes Berg
@ 2023-11-29 8:48 ` Edward Adam Davis
2023-11-29 9:15 ` Johannes Berg
0 siblings, 1 reply; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-29 8:48 UTC (permalink / raw)
To: johannes
Cc: davem, eadavis, edumazet, kuba, linux-kernel, linux-wireless,
llvm, nathan, ndesaulniers, netdev, pabeni,
syzbot+62d7eef57b09bfebcd84, syzkaller-bugs, trix
On Wed, 29 Nov 2023 09:33:23 +0100, Johannes Berg wrote:
> > > > [Analysis]
> > > > When ieee80211_get_link_sband() fails to find a valid sband and first checks
> > > > for params in sta_link_apply_parameters(), it will return 0 due to new_link
> > > > being 0, which will lead to an incorrect process after sta_apply_parameters().
> > > >
> > > > [Fix]
> > > > First obtain sband and perform a non null check before checking the params.
> > >
> > > Not sure I can even disagree with that analysis, it seems right, but ...
> > >
> > > > + if (!link || !link_sta)
> > > > + return -EINVAL;
> > > > +
> > > > + sband = ieee80211_get_link_sband(link);
> > > > + if (!sband)
> > > > + return -EINVAL;
> > > > +
> > > > /*
> > > > * If there are no changes, then accept a link that doesn't exist,
> > > > * unless it's a new link.
> > >
> > > There's a comment here which is clearly not true after this change,
> > > since you've already returned for !link_sta?
> > No, after applying my patch, it will return due to !sband.
> >
>
> Right, OK, but the way I read the comment (now) is that it wanted to
> accept it in that case?
>
> That said, I just threw the patch into our internal testing machinery
> quickly (probably has more MLO tests than upstream hostap for now), and
> it worked just fine ...
>
> Maybe we should just remove the comment?
Do you mean to delete the comments below?
3 /*
2 * If there are no changes, then accept a link that doesn't exist,
1 * unless it's a new link.
1800 */
Edward
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] wifi: mac80211: sband's null check should precede params
2023-11-29 8:48 ` Edward Adam Davis
@ 2023-11-29 9:15 ` Johannes Berg
2023-11-29 12:17 ` [PATCH V2] wifi: mac80211: check if the existing link config remains unchanged Edward Adam Davis
0 siblings, 1 reply; 17+ messages in thread
From: Johannes Berg @ 2023-11-29 9:15 UTC (permalink / raw)
To: Edward Adam Davis
Cc: davem, edumazet, kuba, linux-kernel, linux-wireless, llvm, nathan,
ndesaulniers, netdev, pabeni, syzbot+62d7eef57b09bfebcd84,
syzkaller-bugs, trix
On Wed, 2023-11-29 at 16:48 +0800, Edward Adam Davis wrote:
> On Wed, 29 Nov 2023 09:33:23 +0100, Johannes Berg wrote:
> > > > > [Analysis]
> > > > > When ieee80211_get_link_sband() fails to find a valid sband and first checks
> > > > > for params in sta_link_apply_parameters(), it will return 0 due to new_link
> > > > > being 0, which will lead to an incorrect process after sta_apply_parameters().
> > > > >
> > > > > [Fix]
> > > > > First obtain sband and perform a non null check before checking the params.
> > > >
> > > > Not sure I can even disagree with that analysis, it seems right, but ...
> > > >
> > > > > + if (!link || !link_sta)
> > > > > + return -EINVAL;
> > > > > +
> > > > > + sband = ieee80211_get_link_sband(link);
> > > > > + if (!sband)
> > > > > + return -EINVAL;
> > > > > +
> > > > > /*
> > > > > * If there are no changes, then accept a link that doesn't exist,
> > > > > * unless it's a new link.
> > > >
> > > > There's a comment here which is clearly not true after this change,
> > > > since you've already returned for !link_sta?
> > > No, after applying my patch, it will return due to !sband.
> > >
> >
> > Right, OK, but the way I read the comment (now) is that it wanted to
> > accept it in that case?
> >
> > That said, I just threw the patch into our internal testing machinery
> > quickly (probably has more MLO tests than upstream hostap for now), and
> > it worked just fine ...
> >
> > Maybe we should just remove the comment?
> Do you mean to delete the comments below?
> 3 /*
> 2 * If there are no changes, then accept a link that doesn't exist,
> 1 * unless it's a new link.
> 1800 */
>
Right, it doesn't seem correct any more?
johannes
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH V2] wifi: mac80211: check if the existing link config remains unchanged
2023-11-29 9:15 ` Johannes Berg
@ 2023-11-29 12:17 ` Edward Adam Davis
0 siblings, 0 replies; 17+ messages in thread
From: Edward Adam Davis @ 2023-11-29 12:17 UTC (permalink / raw)
To: johannes
Cc: davem, eadavis, edumazet, kuba, linux-kernel, linux-wireless,
llvm, nathan, ndesaulniers, netdev, pabeni,
syzbot+62d7eef57b09bfebcd84, syzkaller-bugs, trix
[Syz report]
WARNING: CPU: 1 PID: 5067 at net/mac80211/rate.c:48 rate_control_rate_init+0x540/0x690 net/mac80211/rate.c:48
Modules linked in:
CPU: 1 PID: 5067 Comm: syz-executor413 Not tainted 6.7.0-rc3-syzkaller-00014-gdf60cee26a2e #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/10/2023
RIP: 0010:rate_control_rate_init+0x540/0x690 net/mac80211/rate.c:48
Code: 48 c7 c2 00 46 0c 8c be 08 03 00 00 48 c7 c7 c0 45 0c 8c c6 05 70 79 0b 05 01 e8 1b a0 6f f7 e9 e0 fd ff ff e8 61 b3 8f f7 90 <0f> 0b 90 e9 36 ff ff ff e8 53 b3 8f f7 e8 5e 0b 78 f7 31 ff 89 c3
RSP: 0018:ffffc90003c57248 EFLAGS: 00010293
RAX: 0000000000000000 RBX: ffff888016bc4000 RCX: ffffffff89f7d519
RDX: ffff888076d43b80 RSI: ffffffff89f7d6df RDI: 0000000000000005
RBP: ffff88801daaae20 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000002 R12: 0000000000000001
R13: 0000000000000000 R14: ffff888020030e20 R15: ffff888078f08000
FS: 0000555556b94380(0000) GS:ffff8880b9900000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000005fdeb8 CR3: 0000000076d22000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<TASK>
sta_apply_auth_flags.constprop.0+0x4b7/0x510 net/mac80211/cfg.c:1674
sta_apply_parameters+0xaf1/0x16c0 net/mac80211/cfg.c:2002
ieee80211_add_station+0x3fa/0x6c0 net/mac80211/cfg.c:2068
rdev_add_station net/wireless/rdev-ops.h:201 [inline]
nl80211_new_station+0x13ba/0x1a70 net/wireless/nl80211.c:7603
genl_family_rcv_msg_doit+0x1fc/0x2e0 net/netlink/genetlink.c:972
genl_family_rcv_msg net/netlink/genetlink.c:1052 [inline]
genl_rcv_msg+0x561/0x800 net/netlink/genetlink.c:1067
netlink_rcv_skb+0x16b/0x440 net/netlink/af_netlink.c:2545
genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
netlink_unicast_kernel net/netlink/af_netlink.c:1342 [inline]
netlink_unicast+0x53b/0x810 net/netlink/af_netlink.c:1368
netlink_sendmsg+0x93c/0xe40 net/netlink/af_netlink.c:1910
sock_sendmsg_nosec net/socket.c:730 [inline]
__sock_sendmsg+0xd5/0x180 net/socket.c:745
____sys_sendmsg+0x6ac/0x940 net/socket.c:2584
___sys_sendmsg+0x135/0x1d0 net/socket.c:2638
__sys_sendmsg+0x117/0x1e0 net/socket.c:2667
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
[Analysis]
It is inappropriate to make a link configuration change judgment on an
non-existent and non new link.
[Fix]
Quickly exit when there is a existent link and the link configuration has not
changed.
Fixes: b303835dabe0 ("wifi: mac80211: accept STA changes without link changes")
Reported-and-tested-by: syzbot+62d7eef57b09bfebcd84@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
net/mac80211/cfg.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..d0b5a5dd7410 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1788,10 +1788,10 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
lockdep_is_held(&local->hw.wiphy->mtx));
/*
- * If there are no changes, then accept a link that doesn't exist,
+ * If there are no changes, then accept a link that exist,
* unless it's a new link.
*/
- if (params->link_id < 0 && !new_link &&
+ if (params->link_id >= 0 && !new_link &&
!params->link_mac && !params->txpwr_set &&
!params->supported_rates_len &&
!params->ht_capa && !params->vht_capa &&
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [syzbot] [wireless?] WARNING in rate_control_rate_init
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
` (3 preceding siblings ...)
2023-11-29 5:48 ` [PATCH] wifi: mac80211: sband's null check should precede params Edward Adam Davis
@ 2023-11-29 11:04 ` syzbot
2023-11-29 11:26 ` syzbot
5 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-29 11:04 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: [wireless?] WARNING in rate_control_rate_init
Author: eadavis@qq.com
please test WARNING in rate_control_rate_init
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2332e0ab53
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..d0b5a5dd7410 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1788,10 +1788,10 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
lockdep_is_held(&local->hw.wiphy->mtx));
/*
- * If there are no changes, then accept a link that doesn't exist,
+ * If there are no changes, then accept a link that exist,
* unless it's a new link.
*/
- if (params->link_id < 0 && !new_link &&
+ if ((sta->sta.valid_links & BIT(params->link_id)) && !new_link &&
!params->link_mac && !params->txpwr_set &&
!params->supported_rates_len &&
!params->ht_capa && !params->vht_capa &&
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [syzbot] [wireless?] WARNING in rate_control_rate_init
2023-07-02 15:15 [syzbot] [wireless?] WARNING in rate_control_rate_init (2) syzbot
` (4 preceding siblings ...)
2023-11-29 11:04 ` [syzbot] [wireless?] WARNING in rate_control_rate_init syzbot
@ 2023-11-29 11:26 ` syzbot
5 siblings, 0 replies; 17+ messages in thread
From: syzbot @ 2023-11-29 11:26 UTC (permalink / raw)
To: linux-kernel
For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.
***
Subject: [wireless?] WARNING in rate_control_rate_init
Author: eadavis@qq.com
please test WARNING in rate_control_rate_init
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 6e2332e0ab53
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 606b1b2e4123..d0b5a5dd7410 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1788,10 +1788,10 @@ static int sta_link_apply_parameters(struct ieee80211_local *local,
lockdep_is_held(&local->hw.wiphy->mtx));
/*
- * If there are no changes, then accept a link that doesn't exist,
+ * If there are no changes, then accept a link that exist,
* unless it's a new link.
*/
- if (params->link_id < 0 && !new_link &&
+ if (params->link_id >= 0 && !new_link &&
!params->link_mac && !params->txpwr_set &&
!params->supported_rates_len &&
!params->ht_capa && !params->vht_capa &&
--
2.43.0
^ permalink raw reply related [flat|nested] 17+ messages in thread