[syzbot] [wireless?] WARNING in rate_control_rate_init (2)

[syzbot] [wireless?] WARNING in rate_control_rate_init (2)

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    6e2332e0ab53 Merge tag 'cgroup-for-6.5' of git://git.kerne..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16e1c60b280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=b8f24c1070268858
dashboard link: https://syzkaller.appspot.com/bug?extid=62d7eef57b09bfebcd84
compiler:       gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=171c0767280000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10113ebd280000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-6e2332e0.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/5c6bc163c340/vmlinux-6e2332e0.xz
kernel image: https://storage.googleapis.com/syzbot-assets/f1e705993336/bzImage-6e2332e0.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+62d7eef57b09bfebcd84@syzkaller.appspotmail.com

------------[ cut here ]------------
WARNING: CPU: 0 PID: 5126 at net/mac80211/rate.c:48 rate_control_rate_init+0x548/0x740 net/mac80211/rate.c:48
Modules linked in:
CPU: 0 PID: 5126 Comm: syz-executor279 Not tainted 6.4.0-syzkaller-01647-g6e2332e0ab53 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.14.0-2 04/01/2014
RIP: 0010:rate_control_rate_init+0x548/0x740 net/mac80211/rate.c:48
Code: f7 48 c7 c2 00 84 7f 8b be 09 03 00 00 48 c7 c7 c0 83 7f 8b c6 05 f9 bc d6 04 01 e8 22 ac d6 f7 e9 d8 fd ff ff e8 a8 16 f6 f7 <0f> 0b e8 c1 32 83 00 31 ff 89 c3 89 c6 e8 b6 12 f6 f7 85 db 75 27
RSP: 0018:ffffc90003197280 EFLAGS: 00010293
RAX: 0000000000000000 RBX: ffff8881070796c0 RCX: 0000000000000000
RDX: ffff88802a51cb80 RSI: ffffffff898db228 RDI: 0000000000000005
RBP: ffff8880255c0000 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000001 R11: 0000000000000001 R12: 0000000000000001
R13: 0000000000000000 R14: ffff888021f30de0 R15: ffff888032530000
FS:  000055555570f300(0000) GS:ffff88806b600000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000020000080 CR3: 000000001f594000 CR4: 0000000000350ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
 <TASK>
 sta_apply_auth_flags.constprop.0+0x424/0x4a0 net/mac80211/cfg.c:1678
 sta_apply_parameters+0xaf8/0x16f0 net/mac80211/cfg.c:2005
 ieee80211_add_station+0x3d0/0x620 net/mac80211/cfg.c:2070
 rdev_add_station net/wireless/rdev-ops.h:201 [inline]
 nl80211_new_station+0x1258/0x1b20 net/wireless/nl80211.c:7564
 genl_family_rcv_msg_doit.isra.0+0x1e6/0x2d0 net/netlink/genetlink.c:968
 genl_family_rcv_msg net/netlink/genetlink.c:1048 [inline]
 genl_rcv_msg+0x4ff/0x7e0 net/netlink/genetlink.c:1065
 netlink_rcv_skb+0x165/0x440 net/netlink/af_netlink.c:2546
 genl_rcv+0x28/0x40 net/netlink/genetlink.c:1076
 netlink_unicast_kernel net/netlink/af_netlink.c:1339 [inline]
 netlink_unicast+0x547/0x7f0 net/netlink/af_netlink.c:1365
 netlink_sendmsg+0x925/0xe30 net/netlink/af_netlink.c:1913
 sock_sendmsg_nosec net/socket.c:725 [inline]
 sock_sendmsg+0xde/0x190 net/socket.c:748
 ____sys_sendmsg+0x722/0x900 net/socket.c:2504
 ___sys_sendmsg+0x110/0x1b0 net/socket.c:2558
 __sys_sendmsg+0xf7/0x1c0 net/socket.c:2587
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7fc033504a69
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 14 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe0868f2d8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 000000000000ae5a RCX: 00007fc033504a69
RDX: 0000000000000000 RSI: 0000000020000280 RDI: 0000000000000004
RBP: 0000000000000000 R08: 00007ffe0868f478 R09: 00007ffe0868f478
R10: 0000000000000000 R11: 0000000000000246 R12: 00007ffe0868f2ec
R13: 431bde82d7b634db R14: 0000000000000000 R15: 0000000000000000
 </TASK>


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the bug is already fixed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to change bug's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the bug is a duplicate of another bug, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [wireless?] WARNING in rate_control_rate_init (2)

[syzbot]

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

Re: [syzbot] [wireless?] WARNING in rate_control_rate_init

[syzbot]

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,
 					&params->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)) {

Re: [syzbot] [wireless?] WARNING in rate_control_rate_init

[syzbot]

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,
 					&params->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)) {

Re: [PATCH] wifi: mac80211: sband's null check should precede params

[Johannes Berg]

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

Re: [PATCH] wifi: mac80211: sband's null check should precede params

[Johannes Berg]

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

Re: [PATCH] wifi: mac80211: sband's null check should precede params

[Johannes Berg]

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

Re: [syzbot] [wireless?] WARNING in rate_control_rate_init

[syzbot]

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

Re: [syzbot] [wireless?] WARNING in rate_control_rate_init

[syzbot]

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