[PATCH] cfg80211: fix double-free after changing network namespace

[PATCH] cfg80211: fix double-free after changing network namespace

[Stefan Bühler]

From: Stefan Bühler <source@stbuehler.de>

If wdev->wext.keys was initialized it didn't get reset to NULL on
unregister (and it doesn't get set in cfg80211_init_wdev either), but
wdev is reused if unregister was triggered through
cfg80211_switch_netns.

The next unregister (for whatever reason) will try to free
wdev->wext.keys again.

Signed-off-by: Stefan Bühler <source@stbuehler.de>
---
 net/wireless/core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 350513744575..3e25229a059d 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1102,6 +1102,7 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
 
 #ifdef CONFIG_CFG80211_WEXT
 	kzfree(wdev->wext.keys);
+	wdev->wext.keys = NULL;
 #endif
 	/* only initialized if we have a netdev */
 	if (wdev->netdev)
-- 
2.24.0

Re: [PATCH] cfg80211: fix double-free after changing network namespace

[Stefan Bühler]

Hi,

I'd also like to see this backported to stable, but
submitting-patches.rst says you don't like individual developers adding
the tag :)

Just for additional context, the KASAN log looks like this:

---
video: module verification failed: signature and/or required key missing
- tainting kernel
[...]
==================================================================
BUG: KASAN: use-after-free in ksize+0x20/0x60
Read of size 1 at addr ffff888152eb6000 by task ns-start/1947

CPU: 2 PID: 1947 Comm: ns-start Tainted: G            E     5.3.9-falcot #1
Hardware name: Intel(R) Client Systems NUC8i3BEK/NUC8BEB, BIOS
BECFL357.86A.0075.2019.1023.1448 10/23/2019
Call Trace:
 dump_stack+0x71/0xa0
 print_address_description.cold+0xd3/0x313
 ? ksize+0x20/0x60
 ? ksize+0x20/0x60
 __kasan_report.cold+0x1a/0x3d
 ? ksize+0x20/0x60
 kasan_report+0xe/0x12
 check_memory_region+0x11b/0x1a0
 ksize+0x20/0x60
 kzfree+0x14/0x30
 __cfg80211_unregister_wdev+0x1dc/0x380 [cfg80211]
 cfg80211_netdev_notifier_call+0x9d9/0x1240 [cfg80211]
 ? addrconf_ifdown+0xbcf/0xf00
 ? cfg80211_init_wdev+0x4c0/0x4c0 [cfg80211]
 ? addrconf_notify+0x11f/0x2050
 ? _raw_spin_lock+0xd0/0xd0
 ? mutex_lock+0x8e/0xe0
 ? __mutex_lock_slowpath+0x10/0x10
 ? inet6_ifinfo_notify+0x100/0x100
 ? mutex_unlock+0x1d/0x40
 notifier_call_chain+0xbe/0x130
 rollback_registered_many+0x686/0xb50
 ? unlist_netdevice+0x3e0/0x3e0
 ? free_one_page+0x78/0x1c0
 ? mutex_lock+0x8e/0xe0
 ? __mutex_lock_slowpath+0x10/0x10
 unregister_netdevice_many.part.0+0x13/0x1c0
 ieee80211_remove_interfaces+0x31f/0x760 [mac80211]
 ? kfree_call_rcu+0x10/0x10
 ? _raw_spin_lock_irqsave+0x8d/0xf0
 ? ieee80211_sdata_stop+0x70/0x70 [mac80211]
 ? mutex_lock+0x8e/0xe0
 ieee80211_unregister_hw+0x47/0x200 [mac80211]
 iwl_op_mode_mvm_stop+0x8b/0x5e0 [iwlmvm]
 _iwl_op_mode_stop.isra.0+0x74/0xc0 [iwlwifi]
 iwl_drv_stop+0x2e/0x560 [iwlwifi]
 iwl_pci_remove+0x8d/0xe0 [iwlwifi]
 pci_device_remove+0xf3/0x290
 ? pcibios_free_irq+0x10/0x10
 ? up_read+0x15/0x90
 device_release_driver_internal+0x1ad/0x440
 pci_stop_bus_device+0x123/0x190
 pci_stop_and_remove_bus_device_locked+0x16/0x30
 remove_store+0xcb/0xe0
 ? sriov_numvfs_store+0x250/0x250
 kernfs_fop_write+0x260/0x410
 ? security_file_permission+0x6e/0x2c0
 ? do_fcntl+0x48f/0x8d0
 vfs_write+0x14e/0x450
 ksys_write+0xed/0x1c0
 ? __ia32_sys_read+0xb0/0xb0
 ? fput_many+0x1c/0x130
 do_syscall_64+0x9c/0x2f0
 ? prepare_exit_to_usermode+0xe8/0x170
 entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x7f8d98487904
Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb bb 0f 1f 80 00 00 00 00
48 8d 05 d9 3a 0d 00 8b 00 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00
f0 ff ff 77 54 c3 0f 1f 00 48 83 ec 28 48 89 54 24 18 48
RSP: 002b:00007ffebd049408 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000002 RCX: 00007f8d98487904
RDX: 0000000000000002 RSI: 000056308a2a79e0 RDI: 0000000000000001
RBP: 000056308a2a79e0 R08: 00000000ffffffff R09: 000000000000000a
R10: 000056308a2af790 R11: 0000000000000246 R12: 0000000000000002
R13: 00007f8d98556760 R14: 0000000000000002 R15: 00007f8d98556960

Allocated by task 1779:
 save_stack+0x1b/0x80
 __kasan_kmalloc.constprop.0+0xc2/0xd0
 __cfg80211_set_encryption+0xc87/0x1bd0 [cfg80211]
 cfg80211_wext_siwencodeext+0x414/0xa20 [cfg80211]
 ioctl_standard_iw_point+0x6b0/0x9e0
 ioctl_standard_call+0x12d/0x160
 wext_handle_ioctl+0xeb/0x170
 sock_ioctl+0x3b0/0x5f0
 do_vfs_ioctl+0x9a1/0xf10
 ksys_ioctl+0x5e/0x90
 __x64_sys_ioctl+0x6f/0xb0
 do_syscall_64+0x9c/0x2f0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

Freed by task 1878:
 save_stack+0x1b/0x80
 __kasan_slab_free+0x117/0x160
 kfree+0x86/0x260
 __cfg80211_unregister_wdev+0x1dc/0x380 [cfg80211]
 cfg80211_netdev_notifier_call+0x9d9/0x1240 [cfg80211]
 notifier_call_chain+0xbe/0x130
 dev_change_net_namespace+0x1cd/0xb00
 cfg80211_switch_netns+0xf0/0x570 [cfg80211]
 nl80211_wiphy_netns+0x107/0x210 [cfg80211]
 genl_family_rcv_msg+0x50e/0xe50
 genl_rcv_msg+0x9f/0x130
 netlink_rcv_skb+0x128/0x360
 genl_rcv+0x24/0x40
 netlink_unicast+0x3f2/0x5d0
 netlink_sendmsg+0x6c4/0xb10
 sock_sendmsg+0xe4/0x110
 ___sys_sendmsg+0x64e/0x9a0
 __sys_sendmsg+0xbe/0x150
 do_syscall_64+0x9c/0x2f0
 entry_SYSCALL_64_after_hwframe+0x44/0xa9

The buggy address belongs to the object at ffff888152eb6000
                                       which belongs to the cache
kmalloc-192 of size 192
The buggy address is located 0 bytes inside of
                                       192-byte region
[ffff888152eb6000, ffff888152eb60c0)
The buggy address belongs to the page:
page:ffffea00054bad80 refcount:1 mapcount:0 mapping:ffff888155003000
index:0xffff888152eb6000
flags: 0x17fffc000000200(slab)
raw: 017fffc000000200 0000000000000000 0000000100000001 ffff888155003000
raw: ffff888152eb6000 0000000080100002 00000001ffffffff 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
 ffff888152eb5f00: 00 fc fc 00 fc fc 00 fc fc 00 fc fc fb fc fc fb
 ffff888152eb5f80: fc fc 00 fc fc 00 fc fc 00 fc fc fb fc fc fc fc
>ffff888152eb6000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                   ^
 ffff888152eb6080: fb fb fb fb fb fb fb fb fc fc fc fc fc fc fc fc
 ffff888152eb6100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
Disabling lock debugging due to kernel taint
==================================================================
---

And I trigger it by moving a phy to a namespace (using it there with
wpa_supplicant and dhcp), closing the namespace, and then try the same
again.

cheers,
Stefan

-- 
Stefan Bühler    Mail/xmpp: stefan.buehler@tik.uni-stuttgart.de
Netze und Kommunikationssysteme der Universität Stuttgart (NKS)
https://www.tik.uni-stuttgart.de/    Telefon: +49 711 685 60854

Re: [PATCH] cfg80211: fix double-free after changing network namespace

[Kalle Valo]

"Stefan Bühler" <stefan.buehler@tik.uni-stuttgart.de> writes:

> I'd also like to see this backported to stable, but
> submitting-patches.rst says you don't like individual developers adding
> the tag :)

BTW, that rule only applies with net and net-next trees. With wireless
trees we are happy to have the stable tag in the commit itself.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH] cfg80211: fix double-free after changing network namespace

[Stefan Bühler]

Hi,

On 11/26/19 11:05 AM, Stefan Bühler wrote:
> From: Stefan Bühler <source@stbuehler.de>
> 
> If wdev->wext.keys was initialized it didn't get reset to NULL on
> unregister (and it doesn't get set in cfg80211_init_wdev either), but
> wdev is reused if unregister was triggered through
> cfg80211_switch_netns.
> 
> The next unregister (for whatever reason) will try to free
> wdev->wext.keys again.
> 
> Signed-off-by: Stefan Bühler <source@stbuehler.de>
> ---
>  net/wireless/core.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/wireless/core.c b/net/wireless/core.c
> index 350513744575..3e25229a059d 100644
> --- a/net/wireless/core.c
> +++ b/net/wireless/core.c
> @@ -1102,6 +1102,7 @@ static void __cfg80211_unregister_wdev(struct wireless_dev *wdev, bool sync)
>  
>  #ifdef CONFIG_CFG80211_WEXT
>  	kzfree(wdev->wext.keys);
> +	wdev->wext.keys = NULL;
>  #endif
>  	/* only initialized if we have a netdev */
>  	if (wdev->netdev)
> 

Any status update for this?  Anything I can do?  Should I resubmit this
with "Cc: stable@vger.kernel.org"?

cheers,
Stefan

-- 
Stefan Bühler    Mail/xmpp: stefan.buehler@tik.uni-stuttgart.de
Netze und Kommunikationssysteme der Universität Stuttgart (NKS)
https://www.tik.uni-stuttgart.de/    Telefon: +49 711 685 60854

Re: [PATCH] cfg80211: fix double-free after changing network namespace

[Johannes Berg]

On Wed, 2019-12-04 at 09:26 +0100, Stefan Bühler wrote:
> 
> Any status update for this?  Anything I can do?  Should I resubmit this
> with "Cc: stable@vger.kernel.org"?

No, it's fine, but we're in the middle of the merge window, I'm waiting
for some merge backs etc.

johannes