From: Stefan Bühler <[email protected]>
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 <[email protected]>
---
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
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: [email protected]
Netze und Kommunikationssysteme der Universität Stuttgart (NKS)
https://www.tik.uni-stuttgart.de/ Telefon: +49 711 685 60854
"Stefan Bühler" <[email protected]> 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
Hi,
On 11/26/19 11:05 AM, Stefan Bühler wrote:
> From: Stefan Bühler <[email protected]>
>
> 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 <[email protected]>
> ---
> 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: [email protected]"?
cheers,
Stefan
--
Stefan Bühler Mail/xmpp: [email protected]
Netze und Kommunikationssysteme der Universität Stuttgart (NKS)
https://www.tik.uni-stuttgart.de/ Telefon: +49 711 685 60854
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: [email protected]"?
No, it's fine, but we're in the middle of the merge window, I'm waiting
for some merge backs etc.
johannes