[PATCH 0/2] wifi: mac80211_hwsim: fix netlink release and radio removal races

[PATCH 0/2] wifi: mac80211_hwsim: fix netlink release and radio removal races

[Zhao Li]

The first patch ignores release notifications from netlink protocols
other than NETLINK_GENERIC. The second serializes radio-consuming
command handlers with asynchronous radio removal.
LLM assistance was used to review this series.

Zhao Li (2):
  wifi: mac80211_hwsim: ignore non-generic netlink releases
  wifi: mac80211_hwsim: serialize radio removal with frame commands

 .../wireless/virtual/mac80211_hwsim_main.c    | 27 ++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)


base-commit: 25e73f1d6868c759c8eb56cad4e411692c5341b4
-- 
2.56.0

[PATCH 1/2] wifi: mac80211_hwsim: ignore non-generic netlink releases

[Zhao Li]

Closing an unrelated netlink socket can remove mac80211_hwsim radios or
clear the active wmediumd registration when its port ID matches a generic
netlink socket.

mac80211_hwsim_netlink_notify() handles every NETLINK_URELEASE event,
but netlink port IDs are scoped by protocol. The notifier therefore treats
a release from another protocol as a MAC80211_HWSIM socket release.

Ignore release notifications unless they come from NETLINK_GENERIC.

LLM assistance was used to find the bug and review.

Fixes: 7882513bacb1 ("mac80211_hwsim driver support userspace frame tx/rx")
Fixes: e9ed49bf4c2c ("mac80211-hwsim: Add support for HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Validation: A W=1 module build with MODPOST passed. Not
reproduced; source review confirmed that netlink port IDs are scoped by
protocol.
Sparse C=2 passed. Targeted Smatch introduced no new diagnostics.

 drivers/net/wireless/virtual/mac80211_hwsim_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_main.c b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
index b9446577ff49..fcbc7f4355bd 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim_main.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
@@ -7197,7 +7197,7 @@ static int mac80211_hwsim_netlink_notify(struct notifier_block *nb,
 {
 	struct netlink_notify *notify = _notify;
 
-	if (state != NETLINK_URELEASE)
+	if (state != NETLINK_URELEASE || notify->protocol != NETLINK_GENERIC)
 		return NOTIFY_DONE;
 
 	remove_user_radios(notify->portid, hwsim_net_get_netgroup(notify->net));
-- 
2.56.0

[PATCH 2/2] wifi: mac80211_hwsim: serialize radio removal with frame commands

[Zhao Li]

Destroying a radio while a frame, TX status, or PMSR command is in
progress can make the command access the radio after it has been freed.

The command handlers keep using the result of an RCU rhashtable lookup
after the lookup has finished. Socket release and network namespace
teardown can concurrently remove and free that radio.

Serialize non-dump generic netlink command handlers and asynchronous
removal paths with an hwsim-private mutex. Use the same serialization for
virtio dispatch, which invokes the handlers outside generic netlink.

LLM assistance was used to find the bug and review.

Fixes: e9ed49bf4c2c ("mac80211-hwsim: Add support for HWSIM_ATTR_DESTROY_RADIO_ON_CLOSE")
Cc: stable@vger.kernel.org
Signed-off-by: Zhao Li <enderaoelyther@gmail.com>
---
Validation: A W=1 module build with MODPOST passed. A forced
QEMU/KASAN race between HWSIM_CMD_FRAME and socket-close removal changed 13
unique use-after-free reports to 0, with no lockdep splat after the change.
Network-namespace, virtio, TX-status and PMSR paths were not run.
A reproducer is available on request.
Sparse C=2 passed. Targeted Smatch introduced no new diagnostics.

 .../wireless/virtual/mac80211_hwsim_main.c    | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/net/wireless/virtual/mac80211_hwsim_main.c b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
index fcbc7f4355bd..61d0e2c1d00c 100644
--- a/drivers/net/wireless/virtual/mac80211_hwsim_main.c
+++ b/drivers/net/wireless/virtual/mac80211_hwsim_main.c
@@ -673,6 +673,7 @@ static const struct nl80211_vendor_cmd_info mac80211_hwsim_vendor_events[] = {
 };
 
 DEFINE_SPINLOCK(hwsim_radio_lock);
+static DEFINE_MUTEX(hwsim_radio_mutex);
 LIST_HEAD(hwsim_radios);
 static struct rhashtable hwsim_radios_rht;
 static int hwsim_radio_idx;
@@ -6310,6 +6311,7 @@ static void mac80211_hwsim_free(void)
 {
 	struct mac80211_hwsim_data *data;
 
+	mutex_lock(&hwsim_radio_mutex);
 	spin_lock_bh(&hwsim_radio_lock);
 	while ((data = list_first_entry_or_null(&hwsim_radios,
 						struct mac80211_hwsim_data,
@@ -6323,6 +6325,7 @@ static void mac80211_hwsim_free(void)
 		spin_lock_bh(&hwsim_radio_lock);
 	}
 	spin_unlock_bh(&hwsim_radio_lock);
+	mutex_unlock(&hwsim_radio_mutex);
 	class_unregister(&hwsim_class);
 }
 
@@ -7153,12 +7156,28 @@ static const struct genl_small_ops hwsim_ops[] = {
 	},
 };
 
+static int hwsim_genl_pre_doit(const struct genl_split_ops *ops,
+			       struct sk_buff *skb, struct genl_info *info)
+{
+	mutex_lock(&hwsim_radio_mutex);
+
+	return 0;
+}
+
+static void hwsim_genl_post_doit(const struct genl_split_ops *ops,
+				 struct sk_buff *skb, struct genl_info *info)
+{
+	mutex_unlock(&hwsim_radio_mutex);
+}
+
 static struct genl_family hwsim_genl_family __ro_after_init = {
 	.name = "MAC80211_HWSIM",
 	.version = 1,
 	.maxattr = HWSIM_ATTR_MAX,
 	.policy = hwsim_genl_policy,
 	.netnsok = true,
+	.pre_doit = hwsim_genl_pre_doit,
+	.post_doit = hwsim_genl_post_doit,
 	.module = THIS_MODULE,
 	.small_ops = hwsim_ops,
 	.n_small_ops = ARRAY_SIZE(hwsim_ops),
@@ -7172,6 +7191,7 @@ static void remove_user_radios(u32 portid, int netgroup)
 	struct mac80211_hwsim_data *entry, *tmp;
 	LIST_HEAD(list);
 
+	mutex_lock(&hwsim_radio_mutex);
 	spin_lock_bh(&hwsim_radio_lock);
 	list_for_each_entry_safe(entry, tmp, &hwsim_radios, list) {
 		if (entry->destroy_on_close && entry->portid == portid &&
@@ -7189,6 +7209,7 @@ static void remove_user_radios(u32 portid, int netgroup)
 		mac80211_hwsim_del_radio(entry, wiphy_name(entry->hw->wiphy),
 					 NULL);
 	}
+	mutex_unlock(&hwsim_radio_mutex);
 }
 
 static int mac80211_hwsim_netlink_notify(struct notifier_block *nb,
@@ -7248,6 +7269,7 @@ static void __net_exit hwsim_exit_net(struct net *net)
 	struct mac80211_hwsim_data *data, *tmp;
 	LIST_HEAD(list);
 
+	mutex_lock(&hwsim_radio_mutex);
 	spin_lock_bh(&hwsim_radio_lock);
 	list_for_each_entry_safe(data, tmp, &hwsim_radios, list) {
 		if (!net_eq(wiphy_net(data->hw->wiphy), net))
@@ -7270,6 +7292,7 @@ static void __net_exit hwsim_exit_net(struct net *net)
 					 wiphy_name(data->hw->wiphy),
 					 NULL);
 	}
+	mutex_unlock(&hwsim_radio_mutex);
 
 	ida_free(&hwsim_netgroup_ida, hwsim_net_get_netgroup(net));
 }
@@ -7364,7 +7387,9 @@ static void hwsim_virtio_rx_work(struct work_struct *work)
 	skb_reset_tail_pointer(skb);
 	len = min(len, skb_end_offset(skb));
 	skb_put(skb, len);
+	mutex_lock(&hwsim_radio_mutex);
 	hwsim_virtio_handle_cmd(skb);
+	mutex_unlock(&hwsim_radio_mutex);
 
 	spin_lock_irqsave(&hwsim_virtio_lock, flags);
 	if (!hwsim_virtio_enabled) {
-- 
2.56.0