Re: [PATCH] wifi: virt_wifi: avoid reporting connection success with wrong SSID
From: En-Wei WU
Date: Wed Jul 03 2024 - 23:13:01 EST
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@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: En-Wei Wu <en-wei.wu@xxxxxxxxxxxxx>
> ---
> 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
>