All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ibrahim Hashimov <security@auditcode.ai>
To: alex.aring@gmail.com, stefan@datenfreihafen.org,
	miquel.raynal@bootlin.com
Cc: horms@kernel.org, kuba@kernel.org, linux-wpan@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH net v2] mac802154: lock rx_mac_cmd_list and drain it before freeing sdata
Date: Wed, 22 Jul 2026 12:16:08 +0200	[thread overview]
Message-ID: <20260722101608.37744-1-security@auditcode.ai> (raw)

rx_mac_cmd_list has no locking. The RX softirq producer in
ieee802154_subif_frame() list_add_tail()s while the mac_wq worker
list_del()s, and the worker dereferences mac_pkt->sdata with no
liveness check.

Removing an interface (or the phy) thus frees its sdata while a queued
packet still points at it, and a later worker run reads freed memory: a
KASAN use-after-free, reproduced by flooding a victim interface with MAC
command frames and deleting it. Meanwhile sibling interfaces on the same
phy keep adding to the list, corrupting it.

Add rx_mac_cmd_lock around every access -- producer, worker (which now
dequeues under the lock and processes the packet after) and the new
mac802154_flush_queued_mac_cmds(). Drain the queue before the sdata is
freed: from ieee802154_if_remove() after the RCU grace period (filtered
to that sdata) and from ieee802154_unregister_hw(). The drain precedes
cancel_work_sync() so a run that already dequeued a matching packet is
waited out while its sdata is still alive.

Fixes: d021d218f6d9 ("mac802154: Handle received BEACON_REQ")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: v1 only added mac802154_flush_queued_mac_cmds() guarded by
    cancel_work_sync(), which cannot fix a race whose corrupting party is
    the lockless softirq producer (not a work item), as the review of v1
    pointed out. Add rx_mac_cmd_lock around every list access and drain
    before cancel_work_sync(). rx_beacon_list has the same lockless
    pattern, but its worker never dereferences sdata, so it is left as a
    separate follow-up to keep this fix minimal.

diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 8f2bff268392..b497b5abab0a 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -74,6 +74,10 @@ struct ieee802154_local {
 	struct work_struct rx_beacon_work;
 	struct list_head rx_mac_cmd_list;
 	struct work_struct rx_mac_cmd_work;
+	/* protects rx_mac_cmd_list against the RX softirq producer, the
+	 * mac_wq worker and the teardown flush running concurrently
+	 */
+	spinlock_t rx_mac_cmd_lock;
 
 	/* Association */
 	struct ieee802154_pan_device *assoc_dev;
@@ -300,6 +304,8 @@ static inline bool mac802154_is_beaconing(struct ieee802154_local *local)
 }
 
 void mac802154_rx_mac_cmd_worker(struct work_struct *work);
+void mac802154_flush_queued_mac_cmds(struct ieee802154_local *local,
+				     struct ieee802154_sub_if_data *sdata);
 
 int mac802154_perform_association(struct ieee802154_sub_if_data *sdata,
 				  struct ieee802154_pan_device *coord,
diff --git a/net/mac802154/iface.c b/net/mac802154/iface.c
index b823720630e7..82c86f253b5d 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -694,6 +694,13 @@ void ieee802154_if_remove(struct ieee802154_sub_if_data *sdata)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	synchronize_rcu();
+
+	/* After the grace period no new rx_mac_cmd_list entry can point at
+	 * @sdata; drop the ones already queued before it is freed below, as
+	 * the mac_wq worker derefs mac_pkt->sdata with no liveness check.
+	 */
+	mac802154_flush_queued_mac_cmds(sdata->local, sdata);
+
 	unregister_netdevice(sdata->dev);
 }
 
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index ea1efef3572a..24d1e398d3d9 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -91,6 +91,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops)
 	INIT_LIST_HEAD(&local->interfaces);
 	INIT_LIST_HEAD(&local->rx_beacon_list);
 	INIT_LIST_HEAD(&local->rx_mac_cmd_list);
+	spin_lock_init(&local->rx_mac_cmd_lock);
 	mutex_init(&local->iflist_mtx);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);
@@ -277,6 +278,12 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 	tasklet_kill(&local->tasklet);
 	flush_workqueue(local->workqueue);
 
+	/* Drain rx_mac_cmd_list before ieee802154_remove_interfaces()
+	 * frees every sdata: the mac_wq worker derefs mac_pkt->sdata and
+	 * is not covered by the flush above (that is the data workqueue).
+	 */
+	mac802154_flush_queued_mac_cmds(local, NULL);
+
 	rtnl_lock();
 
 	ieee802154_remove_interfaces(local);
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index cd8f2a11920d..c869c1c0c65f 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -76,8 +76,12 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work)
 	u8 mac_cmd;
 	int rc;
 
+	spin_lock_bh(&local->rx_mac_cmd_lock);
 	mac_pkt = list_first_entry_or_null(&local->rx_mac_cmd_list,
 					   struct cfg802154_mac_pkt, node);
+	if (mac_pkt)
+		list_del(&mac_pkt->node);
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
 	if (!mac_pkt)
 		return;
 
@@ -123,11 +127,48 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work)
 	}
 
 out:
-	list_del(&mac_pkt->node);
 	kfree_skb(mac_pkt->skb);
 	kfree(mac_pkt);
 }
 
+/**
+ * mac802154_flush_queued_mac_cmds - drop pending rx_mac_cmd_list work
+ * @local: the mac802154 device the queue belongs to
+ * @sdata: interface being torn down, or %NULL to flush unconditionally
+ *
+ * Each queued &struct cfg802154_mac_pkt stashes a raw pointer to the
+ * interface it was received on, which mac802154_rx_mac_cmd_worker() later
+ * dereferences without checking whether that interface is still alive.
+ * Callers must invoke this before freeing @sdata (or before freeing every
+ * interface, when @sdata is %NULL) so the worker never runs against freed
+ * memory. The list is drained under rx_mac_cmd_lock, then cancel_work_sync()
+ * waits out a run that already dequeued a matching packet before the drain.
+ */
+void mac802154_flush_queued_mac_cmds(struct ieee802154_local *local,
+				     struct ieee802154_sub_if_data *sdata)
+{
+	struct cfg802154_mac_pkt *mac_pkt, *tmp;
+
+	spin_lock_bh(&local->rx_mac_cmd_lock);
+	list_for_each_entry_safe(mac_pkt, tmp, &local->rx_mac_cmd_list, node) {
+		if (sdata && mac_pkt->sdata != sdata)
+			continue;
+
+		list_del(&mac_pkt->node);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
+
+	cancel_work_sync(&local->rx_mac_cmd_work);
+
+	/* Other interfaces on @local may still have entries pending. */
+	spin_lock_bh(&local->rx_mac_cmd_lock);
+	if (!list_empty(&local->rx_mac_cmd_list))
+		queue_work(local->mac_wq, &local->rx_mac_cmd_work);
+	spin_unlock_bh(&local->rx_mac_cmd_lock);
+}
+
 static int
 ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		       struct sk_buff *skb, const struct ieee802154_hdr *hdr)
@@ -233,7 +274,11 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 
 		mac_pkt->skb = skb_get(skb);
 		mac_pkt->sdata = sdata;
+
+		spin_lock(&sdata->local->rx_mac_cmd_lock);
 		list_add_tail(&mac_pkt->node, &sdata->local->rx_mac_cmd_list);
+		spin_unlock(&sdata->local->rx_mac_cmd_lock);
+
 		queue_work(sdata->local->mac_wq, &sdata->local->rx_mac_cmd_work);
 		return NET_RX_SUCCESS;
 
-- 
2.50.1 (Apple Git-155)

             reply	other threads:[~2026-07-22 10:16 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 10:16 Ibrahim Hashimov [this message]
2026-07-24  9:31 ` [PATCH net v2] mac802154: lock rx_mac_cmd_list and drain it before freeing sdata Miquel Raynal
2026-07-24 21:58 ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260722101608.37744-1-security@auditcode.ai \
    --to=security@auditcode.ai \
    --cc=alex.aring@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wpan@vger.kernel.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=stefan@datenfreihafen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.