[PATCH] wifi: mac80211_hwsim: remove radios from rhashtable before freeing

[PATCH] wifi: mac80211_hwsim: remove radios from rhashtable before freeing

[Cen Zhang]

mac80211_hwsim_new_radio() publishes each registered radio on the
hwsim_radios list and in hwsim_radios_rht. The generic-netlink and
virtio command paths use the rhashtable to find radios by address.

Most radio removal paths remove the hash entry while holding
hwsim_radio_lock before unregistering and freeing the radio. However,
mac80211_hwsim_free() only removes the list entry. During init error
unwinding after hwsim netlink and virtio registration, this can leave a
freed radio reachable from hwsim_radios_rht until the callback surfaces
are unpublished and the rhashtable is destroyed.

The buggy scenario involves two paths, with each column showing the order
within that path:

init error unwind path:              hwsim command path:
1. create and hash a radio           1. receive a command by address
2. hit a later init failure          2. look up hwsim_radios_rht
3. call mac80211_hwsim_free()        3. get the stale radio pointer
4. free the radio                    4. dereference the freed radio
5. unregister netlink and virtio

Remove each radio from hwsim_radios_rht in mac80211_hwsim_free(),
matching the other radio removal paths, before releasing the lock and
freeing the hw object.

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in memcmp+0x1ab/0x1d0

Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0xa0
 print_report+0xce/0x630
 ? memcmp+0x1ab/0x1d0
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __virt_addr_valid+0x224/0x430
 ? memcmp+0x1ab/0x1d0
 kasan_report+0xac/0xe0
 ? memcmp+0x1ab/0x1d0
 memcmp+0x1ab/0x1d0
 get_hwsim_data_ref_from_addr+0x15b/0x4d0 [mac80211_hwsim]
 hwsim_cloned_frame_received_nl+0x1ff/0xce0 [mac80211_hwsim]
 ? __pfx_hwsim_cloned_frame_received_nl+0x10/0x10 [mac80211_hwsim]
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? kasan_save_track+0x14/0x30
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __kasan_kmalloc+0xaa/0xb0
 ? __nla_parse+0x24/0x30
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? genl_family_rcv_msg_attrs_parse.isra.0+0x17f/0x290
 genl_family_rcv_msg_doit+0x1e5/0x2c0
 ? __pfx_genl_family_rcv_msg_doit+0x10/0x10
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? kasan_save_stack+0x42/0x60
 ? kasan_save_stack+0x33/0x60
 ? kasan_save_track+0x14/0x30
 genl_rcv_msg+0x432/0x6f0
 ? __pfx_genl_rcv_msg+0x10/0x10
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __pfx_hwsim_cloned_frame_received_nl+0x10/0x10 [mac80211_hwsim]
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __lock_acquire+0x466/0x2260
 netlink_rcv_skb+0x124/0x350
 ? __pfx_genl_rcv_msg+0x10/0x10
 ? __pfx_netlink_rcv_skb+0x10/0x10
 ? lock_acquire+0x187/0x300
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? netlink_deliver_tap+0x150/0xac0
 genl_rcv+0x28/0x40
 netlink_unicast+0x47c/0x790
 ? __pfx_netlink_unicast+0x10/0x10
 netlink_sendmsg+0x767/0xc30
 ? __pfx_netlink_sendmsg+0x10/0x10
 ? lock_release+0xc8/0x290
 __sys_sendto+0x34f/0x3a0
 ? __pfx___sys_sendto+0x10/0x10
 ? lockdep_hardirqs_on_prepare+0xea/0x1a0
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? __x64_sys_poll+0x181/0x3e0
 ? __pfx___x64_sys_poll+0x10/0x10
 __x64_sys_sendto+0xe0/0x1c0
 ? srso_alias_return_thunk+0x5/0xfbef5
 ? trace_hardirqs_on+0x1a/0x170
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Allocated by task 444:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 __kasan_kmalloc+0xaa/0xb0
 __kmalloc_noprof+0x292/0x770
 p9_fcall_init+0xe5/0x400
 p9_tag_alloc+0x1b8/0x700
 p9_client_prepare_req+0x107/0x3e0
 p9_client_zc_rpc.constprop.0+0xf1/0x860
 p9_client_write+0x36d/0x780
 v9fs_issue_write+0xdd/0x170
 netfs_unbuffered_write+0x339/0x2680
 netfs_unbuffered_write_iter_locked+0x6c4/0x960
 netfs_unbuffered_write_iter+0x2d5/0x540
 vfs_write+0x5fb/0x1230
 ksys_write+0xf9/0x1d0
 do_syscall_64+0x115/0x6a0
 entry_SYSCALL_64_after_hwframe+0x77/0x7f

Freed by task 444:
 kasan_save_stack+0x33/0x60
 kasan_save_track+0x14/0x30
 kasan_save_free_info+0x3b/0x60

Fixes: c6509cc3b3e8 ("mac80211_hwsim: add hashtable with mac address keys for faster lookup")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 drivers/net/wireless/virtual/mac80211_hwsim_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_main.c b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
index 0dd8a6c85953..8e83ebdf4563 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim_main.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
@@ -6274,6 +6274,9 @@ static void mac80211_hwsim_free(void)
 						struct mac80211_hwsim_data,
 						list))) {
 		list_del(&data->list);
+		rhashtable_remove_fast(&hwsim_radios_rht, &data->rht,
+				       hwsim_rht_params);
+		hwsim_radios_generation++;
 		spin_unlock_bh(&hwsim_radio_lock);
 		mac80211_hwsim_del_radio(data, wiphy_name(data->hw->wiphy),
 					 NULL);
-- 
2.43.0

Re: [PATCH] wifi: mac80211_hwsim: remove radios from rhashtable before freeing

[Johannes Berg]

On Sat, 2026-06-20 at 00:24 +0800, Cen Zhang wrote:
> mac80211_hwsim_new_radio() publishes each registered radio on the
> hwsim_radios list and in hwsim_radios_rht. The generic-netlink and
> virtio command paths use the rhashtable to find radios by address.
> 
> Most radio removal paths remove the hash entry while holding
> hwsim_radio_lock before unregistering and freeing the radio. However,
> mac80211_hwsim_free() only removes the list entry. During init error
> unwinding after hwsim netlink and virtio registration, this can leave a
> freed radio reachable from hwsim_radios_rht until the callback surfaces
> are unpublished and the rhashtable is destroyed.

How does the init even fail? I think this is fairly contrived,
especially in a test tool. The whole thing is maybe worth a cleanup, but
the whole scary commit message is nonsense.

johannes

Re: [PATCH] wifi: mac80211_hwsim: remove radios from rhashtable before freeing

[Cen Zhang]

Hi Johannes,

> How does the init even fail? I think this is fairly contrived,
> especially in a test tool. The whole thing is maybe worth a cleanup, but
> the whole scary commit message is nonsense.
>
> johannes

Thanks for your review.

The init failure I had in mind is a late one after radios have already
been created and inserted into hwsim_radios_rht, for example in the
monitor netdev setup after the radio loop. My local validation used
fault injection at dev_alloc_name() plus a bounded debug delay to make
that unwind window observable.

That means the test shows the cleanup ordering issue, but it is indeed
contrived, especially for mac80211_hwsim. The changelog I sent made this
sound much scarier than it should have, and including the long KASAN
report was not helpful.

The actual change I wanted is just the small consistency cleanup:
mac80211_hwsim_free() should remove data->rht before freeing the radio,
like the other radio removal paths already do.

Best regards,
Cen Zhang

Re: [PATCH] wifi: mac80211_hwsim: remove radios from rhashtable before freeing

[Johannes Berg]

On Mon, 2026-07-06 at 20:10 +0800, Cen Zhang wrote:
> Hi Johannes,
> 
> > How does the init even fail? I think this is fairly contrived,
> > especially in a test tool. The whole thing is maybe worth a cleanup, but
> > the whole scary commit message is nonsense.
> > 
> > johannes
> 
> Thanks for your review.
> 
> The init failure I had in mind is a late one after radios have already
> been created and inserted into hwsim_radios_rht, for example in the
> monitor netdev setup after the radio loop. My local validation used
> fault injection at dev_alloc_name() plus a bounded debug delay to make
> that unwind window observable.

Yeah, I guess it _could_ technically happen, though I think the memory
allocation failures should be virtually impossible here. And it's hwsim
after all.

> That means the test shows the cleanup ordering issue, but it is indeed
> contrived, especially for mac80211_hwsim. The changelog I sent made this
> sound much scarier than it should have, and including the long KASAN
> report was not helpful.

Fair.

> The actual change I wanted is just the small consistency cleanup:
> mac80211_hwsim_free() should remove data->rht before freeing the radio,
> like the other radio removal paths already do.

I'd apply this, or maybe a reverse of ordering (making things visible
only after the radios exist), but please resend.

Also, if it's going to be this version, we don't really need
hwsim_radios_generation I think, anyway it's all going away so
consistency to userspace is effectively already dead at that point.

johannes