Closing the socket that owns a destroy-on-close radio can free the radio while a frame, TX status, or PMSR command still uses the pointer returned by get_hwsim_data_ref_from_addr(). A forced QEMU/KASAN run reproduced the resulting use-after-free in hwsim_cloned_frame_received_nl(). rhashtable_lookup_fast() protects only the lookup itself. The hwsim deletion paths already unpublish a radio from the hash table under hwsim_radio_lock before performing the sleeping teardown. Take a per-radio reference under hwsim_radio_lock when an address lookup succeeds. After unpublishing a radio, drop its publication reference and wait for lookup users before unregistering the hardware. This avoids serializing unrelated radios with asynchronous teardown while preventing teardown from freeing an active lookup result. AI assistance was used. Fixes: e9ed49bf4c2c ("mac80211-hwsim: Add support for HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE") Link: https://patch.msgid.link/20261028141751.6728-3-enderaoelyther@gmail.com Cc: stable@vger.kernel.org Signed-off-by: Zhao Li --- Changes in v3: - Resend with git send-email. v2 went out through a Gmail connector that line-wrapped the diff, so it does not apply. No code changes. v2: https://lore.kernel.org/linux-wireless/CACHPgzxg0i5S5hKyuSLjNYhLKr7ZoxKO=VefOtsiU6zgod=yfg@mail.gmail.com/ Changes in v2: - Replace the family-wide command/removal mutex with per-radio lookup references. - Unpublish before dropping the publication reference, then drain lookup users before hardware teardown. - Cover all four lookup sites and make this a standalone patch on wireless/main, where patch 1/2 is already merged. Testing: - GCC 12.2 W=1 module/built-in builds; sparse, Smatch and Coccinelle. - QEMU/KASAN/lockdep forced removal at all four lookup sites. Every base contrast reproduced the use-after-free; the fixed revision stayed clean and passed per-radio isolation, real-cookie TX status and teardown churn. - Not covered: PMSR normal completion, virtio dispatch and module exit. .../net/wireless/virtual/mac80211_hwsim_i.h | 4 ++ .../wireless/virtual/mac80211_hwsim_main.c | 47 +++++++++++++++---- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_i.h b/drivers/net/wireless/virtual/mac80211_hwsim_i.h index 0f0f2ac6d80e..c91bbd99b5a2 100644 --- a/drivers/net/wireless/virtual/mac80211_hwsim_i.h +++ b/drivers/net/wireless/virtual/mac80211_hwsim_i.h @@ -10,6 +10,8 @@ #ifndef __MAC80211_HWSIM_I_H #define __MAC80211_HWSIM_I_H +#include +#include #include #include "mac80211_hwsim.h" #include "mac80211_hwsim_nan.h" @@ -51,6 +53,8 @@ struct mac80211_hwsim_link_data { struct mac80211_hwsim_data { struct list_head list; struct rhash_head rht; + refcount_t users; + struct completion users_done; struct ieee80211_hw *hw; struct device *dev; struct ieee80211_supported_band bands[NUM_NL80211_BANDS]; diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_main.c b/drivers/net/wireless/virtual/mac80211_hwsim_main.c index 49ec2948c68a..4703b6fc328d 100644 --- a/drivers/net/wireless/virtual/mac80211_hwsim_main.c +++ b/drivers/net/wireless/virtual/mac80211_hwsim_main.c @@ -711,7 +711,21 @@ struct hwsim_radiotap_ack_hdr { static struct mac80211_hwsim_data *get_hwsim_data_ref_from_addr(const u8 *addr) { - return rhashtable_lookup_fast(&hwsim_radios_rht, addr, hwsim_rht_params); + struct mac80211_hwsim_data *data; + + spin_lock_bh(&hwsim_radio_lock); + data = rhashtable_lookup_fast(&hwsim_radios_rht, addr, hwsim_rht_params); + if (data && WARN_ON_ONCE(!refcount_inc_not_zero(&data->users))) + data = NULL; + spin_unlock_bh(&hwsim_radio_lock); + + return data; +} + +static void put_hwsim_data(struct mac80211_hwsim_data *data) +{ + if (data && refcount_dec_and_test(&data->users)) + complete(&data->users_done); } /* MAC80211_HWSIM netlink family */ @@ -4236,11 +4250,15 @@ static int hwsim_pmsr_report_nl(struct sk_buff *msg, struct genl_info *info) if (!hwsim_virtio_enabled) { if (hwsim_net_get_netgroup(genl_info_net(info)) != - data->netgroup) - return -EINVAL; + data->netgroup) { + err = -EINVAL; + goto out_put; + } - if (info->snd_portid != data->wmediumd) - return -EPERM; + if (info->snd_portid != data->wmediumd) { + err = -EPERM; + goto out_put; + } } mutex_lock(&data->mutex); @@ -4280,6 +4298,8 @@ static int hwsim_pmsr_report_nl(struct sk_buff *msg, struct genl_info *info) data->pmsr_request_wdev = NULL; mutex_unlock(&data->mutex); +out_put: + put_hwsim_data(data); return err; } @@ -6140,6 +6160,8 @@ static int mac80211_hwsim_new_radio(struct genl_info *info, /* By default all radios belong to the first group */ data->group = 1; mutex_init(&data->mutex); + refcount_set(&data->users, 1); + init_completion(&data->users_done); data->netgroup = hwsim_net_get_netgroup(net); data->wmediumd = hwsim_net_get_wmediumd(net); @@ -6297,6 +6319,10 @@ static void mac80211_hwsim_del_radio(struct mac80211_hwsim_data *data, const char *hwname, struct genl_info *info) { + /* The radio is no longer published. Drain address lookup users. */ + if (!refcount_dec_and_test(&data->users)) + wait_for_completion(&data->users_done); + hwsim_mcast_del_radio(data->idx, hwname, wiphy_net(data->hw->wiphy), info); debugfs_remove_recursive(data->debugfs); ieee80211_unregister_hw(data->hw); @@ -6432,7 +6458,7 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2, { struct ieee80211_hdr *hdr; - struct mac80211_hwsim_data *data2; + struct mac80211_hwsim_data *data2 = NULL; struct ieee80211_tx_info *txi; struct hwsim_tx_rate *tx_attempts; u64 ret_skb_cookie; @@ -6530,8 +6556,10 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2, txi->flags |= IEEE80211_TX_STAT_NOACK_TRANSMITTED; ieee80211_tx_status_irqsafe(data2->hw, skb); + put_hwsim_data(data2); return 0; out: + put_hwsim_data(data2); return -EINVAL; } @@ -6539,7 +6567,7 @@ static int hwsim_tx_info_frame_received_nl(struct sk_buff *skb_2, static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2, struct genl_info *info) { - struct mac80211_hwsim_data *data2; + struct mac80211_hwsim_data *data2 = NULL, *txdata = NULL; struct ieee80211_rx_status rx_status; struct ieee80211_hdr *hdr; const u8 *dst; @@ -6602,7 +6630,6 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2, info->attrs[HWSIM_ATTR_ADDR_TRANSMITTER]) { u64 cookie = nla_get_u64(info->attrs[HWSIM_ATTR_COOKIE]); struct sk_buff *orig_skb, *found = NULL; - struct mac80211_hwsim_data *txdata; struct ieee80211_tx_info *txi; const u8 *transmitter; unsigned long flags; @@ -6695,11 +6722,15 @@ static int hwsim_cloned_frame_received_nl(struct sk_buff *skb_2, mac80211_hwsim_rx(data2, &rx_status, skb); mutex_unlock(&data2->mutex); + put_hwsim_data(txdata); + put_hwsim_data(data2); return 0; out_unlock: mutex_unlock(&data2->mutex); out: + put_hwsim_data(txdata); + put_hwsim_data(data2); dev_kfree_skb(skb); return -EINVAL; } base-commit: 831e33ad02ff884ecc793c07a9c9bd9e06941fe5 -- 2.56.1