Message ID | 20240704030304.16213-1-en-wei.wu@canonical.com |
---|---|
State | Superseded |
Headers | show |
Series | wifi: virt_wifi: avoid reporting connection success with wrong SSID | expand |
Hi, After applying this patch and testing on the machines of syzbot, the original warning is gone. However, syzbot reported that there is another warning generated: create_new_namespaces+0x425/0x7b0 kernel/nsproxy.c:110 unshare_nsproxy_namespaces+0x124/0x180 kernel/nsproxy.c:228 ksys_unshare+0x619/0xc10 kernel/fork.c:3323 __do_sys_unshare kernel/fork.c:3394 [inline] __se_sys_unshare kernel/fork.c:3392 [inline] __x64_sys_unshare+0x38/0x40 kernel/fork.c:3392 do_syscall_x64 arch/x86/entry/common.c:52 [inline] do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83 entry_SYSCALL_64_after_hwframe+0x77/0x7f ------------[ cut here ]------------ WARNING: CPU: 1 PID: 11 at lib/ref_tracker.c:179 ref_tracker_dir_exit+0x411/0x550 lib/ref_tracker.c:179 Modules linked in: CPU: 1 PID: 11 Comm: kworker/u8:0 Not tainted 6.10.0-rc5-syzkaller-01200-gcda91d5b911a-dirty #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 06/07/2024 Workqueue: netns cleanup_net RIP: 0010:ref_tracker_dir_exit+0x411/0x550 lib/ref_tracker.c:179 Code: 48 8b 1c 24 48 89 df 48 8b 74 24 20 e8 88 f2 ab 06 eb 1a e8 f1 b7 ad fc 48 8b 1c 24 48 89 df 48 8b 74 24 20 e8 70 f2 ab 06 90 <0f> 0b 90 48 83 c3 44 48 89 df be 04 00 00 00 e8 5b 7f 13 fd 48 89 RSP: 0018:ffffc900001079e0 EFLAGS: 00010246 RAX: 2ba763440467a200 RBX: ffff8880296b9ed8 RCX: 0000000000000001 RDX: dffffc0000000000 RSI: ffffffff8bcabb40 RDI: 0000000000000001 RBP: ffffc90000107ab0 R08: ffffffff92fbc66f R09: 1ffffffff25f78cd R10: dffffc0000000000 R11: fffffbfff25f78ce R12: 1ffff1100da640d8 R13: dead000000000100 R14: ffff8880296b9f28 R15: dffffc0000000000 FS: 0000000000000000(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 000055b2700a0950 CR3: 000000002c8b0000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> net_free net/core/net_namespace.c:465 [inline] cleanup_net+0xbf3/0xcc0 net/core/net_namespace.c:661 process_one_work kernel/workqueue.c:3248 [inline] process_scheduled_works+0xa2c/0x1830 kernel/workqueue.c:3329 worker_thread+0x86d/0xd50 kernel/workqueue.c:3409 kthread+0x2f0/0x390 kernel/kthread.c:389 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:244 </TASK> It looks like it's not related to the changes I have made, since the new warning is probably describing a leak when doing net_free. I will look at the new warning soon, and maybe we can just focus on the original warning now. En-Wei. On Thu, 4 Jul 2024 at 11:03, En-Wei Wu <en-wei.wu@canonical.com> wrote: > > When user issues a connection with a different SSID than the one > virt_wifi has advertised, the __cfg80211_connect_result() will > trigger the warning: WARN_ON(bss_not_found). > > The issue is because the connection code in virt_wifi does not > check the SSID from user space (it only checks the BSSID), and > virt_wifi will call cfg80211_connect_result() with WLAN_STATUS_SUCCESS > even if the SSID is different from the one virt_wifi has advertised. > Eventually cfg80211 won't be able to find the cfg80211_bss and generate > the warning. > > Fixed it by checking the SSID (from user space) in the connection code. > > Fixes: c7cdba31ed8b ("mac80211-next: rtnetlink wifi simulation device") > Reported-by: syzbot+d6eb9cee2885ec06f5e3@syzkaller.appspotmail.com > Signed-off-by: En-Wei Wu <en-wei.wu@canonical.com> > --- > drivers/net/wireless/virtual/virt_wifi.c | 24 +++++++++++++++++++++--- > 1 file changed, 21 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/virtual/virt_wifi.c b/drivers/net/wireless/virtual/virt_wifi.c > index 6a84ec58d618..86a8f74be654 100644 > --- a/drivers/net/wireless/virtual/virt_wifi.c > +++ b/drivers/net/wireless/virtual/virt_wifi.c > @@ -136,6 +136,9 @@ static struct ieee80211_supported_band band_5ghz = { > /* Assigned at module init. Guaranteed locally-administered and unicast. */ > static u8 fake_router_bssid[ETH_ALEN] __ro_after_init = {}; > > +#define VIRT_WIFI_SSID_LEN 8 > +#define VIRT_WIFI_SSID "VirtWifi" > + > static void virt_wifi_inform_bss(struct wiphy *wiphy) > { > u64 tsf = div_u64(ktime_get_boottime_ns(), 1000); > @@ -146,8 +149,8 @@ static void virt_wifi_inform_bss(struct wiphy *wiphy) > u8 ssid[8]; > } __packed ssid = { > .tag = WLAN_EID_SSID, > - .len = 8, > - .ssid = "VirtWifi", > + .len = VIRT_WIFI_SSID_LEN, > + .ssid = VIRT_WIFI_SSID, > }; > > informed_bss = cfg80211_inform_bss(wiphy, &channel_5ghz, > @@ -213,6 +216,8 @@ struct virt_wifi_netdev_priv { > struct net_device *upperdev; > u32 tx_packets; > u32 tx_failed; > + u32 connect_requested_ssid_len; > + u8 connect_requested_ssid[IEEE80211_MAX_SSID_LEN]; > u8 connect_requested_bss[ETH_ALEN]; > bool is_up; > bool is_connected; > @@ -224,11 +229,21 @@ static int virt_wifi_connect(struct wiphy *wiphy, struct net_device *netdev, > struct cfg80211_connect_params *sme) > { > struct virt_wifi_netdev_priv *priv = netdev_priv(netdev); > + u32 ssid_len; > bool could_schedule; > > if (priv->being_deleted || !priv->is_up) > return -EBUSY; > > + if (!sme->ssid) { > + wiphy_err(wiphy, "invalid SSID\n"); > + return -EINVAL; > + } > + > + ssid_len = min_t(u32, sme->ssid_len, IEEE80211_MAX_SSID_LEN); > + priv->connect_requested_ssid_len = ssid_len; > + memcpy(priv->connect_requested_ssid, sme->ssid, ssid_len); > + > could_schedule = schedule_delayed_work(&priv->connect, HZ * 2); > if (!could_schedule) > return -EBUSY; > @@ -252,12 +267,15 @@ static void virt_wifi_connect_complete(struct work_struct *work) > container_of(work, struct virt_wifi_netdev_priv, connect.work); > u8 *requested_bss = priv->connect_requested_bss; > bool right_addr = ether_addr_equal(requested_bss, fake_router_bssid); > + bool right_ssid = (priv->connect_requested_ssid_len == VIRT_WIFI_SSID_LEN ? > + !memcmp(priv->connect_requested_ssid, VIRT_WIFI_SSID, > + priv->connect_requested_ssid_len) : false); > u16 status = WLAN_STATUS_SUCCESS; > > if (is_zero_ether_addr(requested_bss)) > requested_bss = NULL; > > - if (!priv->is_up || (requested_bss && !right_addr)) > + if (!priv->is_up || (requested_bss && !right_addr) || !right_ssid) > status = WLAN_STATUS_UNSPECIFIED_FAILURE; > else > priv->is_connected = true; > -- > 2.43.0 >
diff --git a/drivers/net/wireless/virtual/virt_wifi.c b/drivers/net/wireless/virtual/virt_wifi.c index 6a84ec58d618..86a8f74be654 100644 --- a/drivers/net/wireless/virtual/virt_wifi.c +++ b/drivers/net/wireless/virtual/virt_wifi.c @@ -136,6 +136,9 @@ static struct ieee80211_supported_band band_5ghz = { /* Assigned at module init. Guaranteed locally-administered and unicast. */ static u8 fake_router_bssid[ETH_ALEN] __ro_after_init = {}; +#define VIRT_WIFI_SSID_LEN 8 +#define VIRT_WIFI_SSID "VirtWifi" + static void virt_wifi_inform_bss(struct wiphy *wiphy) { u64 tsf = div_u64(ktime_get_boottime_ns(), 1000); @@ -146,8 +149,8 @@ static void virt_wifi_inform_bss(struct wiphy *wiphy) u8 ssid[8]; } __packed ssid = { .tag = WLAN_EID_SSID, - .len = 8, - .ssid = "VirtWifi", + .len = VIRT_WIFI_SSID_LEN, + .ssid = VIRT_WIFI_SSID, }; informed_bss = cfg80211_inform_bss(wiphy, &channel_5ghz, @@ -213,6 +216,8 @@ struct virt_wifi_netdev_priv { struct net_device *upperdev; u32 tx_packets; u32 tx_failed; + u32 connect_requested_ssid_len; + u8 connect_requested_ssid[IEEE80211_MAX_SSID_LEN]; u8 connect_requested_bss[ETH_ALEN]; bool is_up; bool is_connected; @@ -224,11 +229,21 @@ static int virt_wifi_connect(struct wiphy *wiphy, struct net_device *netdev, struct cfg80211_connect_params *sme) { struct virt_wifi_netdev_priv *priv = netdev_priv(netdev); + u32 ssid_len; bool could_schedule; if (priv->being_deleted || !priv->is_up) return -EBUSY; + if (!sme->ssid) { + wiphy_err(wiphy, "invalid SSID\n"); + return -EINVAL; + } + + ssid_len = min_t(u32, sme->ssid_len, IEEE80211_MAX_SSID_LEN); + priv->connect_requested_ssid_len = ssid_len; + memcpy(priv->connect_requested_ssid, sme->ssid, ssid_len); + could_schedule = schedule_delayed_work(&priv->connect, HZ * 2); if (!could_schedule) return -EBUSY; @@ -252,12 +267,15 @@ static void virt_wifi_connect_complete(struct work_struct *work) container_of(work, struct virt_wifi_netdev_priv, connect.work); u8 *requested_bss = priv->connect_requested_bss; bool right_addr = ether_addr_equal(requested_bss, fake_router_bssid); + bool right_ssid = (priv->connect_requested_ssid_len == VIRT_WIFI_SSID_LEN ? + !memcmp(priv->connect_requested_ssid, VIRT_WIFI_SSID, + priv->connect_requested_ssid_len) : false); u16 status = WLAN_STATUS_SUCCESS; if (is_zero_ether_addr(requested_bss)) requested_bss = NULL; - if (!priv->is_up || (requested_bss && !right_addr)) + if (!priv->is_up || (requested_bss && !right_addr) || !right_ssid) status = WLAN_STATUS_UNSPECIFIED_FAILURE; else priv->is_connected = true;
When user issues a connection with a different SSID than the one virt_wifi has advertised, the __cfg80211_connect_result() will trigger the warning: WARN_ON(bss_not_found). The issue is because the connection code in virt_wifi does not check the SSID from user space (it only checks the BSSID), and virt_wifi will call cfg80211_connect_result() with WLAN_STATUS_SUCCESS even if the SSID is different from the one virt_wifi has advertised. Eventually cfg80211 won't be able to find the cfg80211_bss and generate the warning. Fixed it by checking the SSID (from user space) in the connection code. Fixes: c7cdba31ed8b ("mac80211-next: rtnetlink wifi simulation device") Reported-by: syzbot+d6eb9cee2885ec06f5e3@syzkaller.appspotmail.com Signed-off-by: En-Wei Wu <en-wei.wu@canonical.com> --- drivers/net/wireless/virtual/virt_wifi.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)