Netdev List
 help / color / mirror / Atom feed
* [PATCH wpan v3] mac802154: fix use-after-free of sdata via queued RX frames
@ 2026-07-25 13:51 Ibrahim Hashimov
  0 siblings, 0 replies; only message in thread
From: Ibrahim Hashimov @ 2026-07-25 13:51 UTC (permalink / raw)
  To: miquel.raynal, alex.aring, stefan
  Cc: kuba, davem, edumazet, pabeni, horms, david.girault, linux-wpan,
	netdev, linux-kernel, stable

The RX softirq producer ieee802154_subif_frame() queues received beacon
and MAC-command frames onto local->rx_beacon_list / rx_mac_cmd_list and
schedules a process-context worker, storing a raw mac_pkt->sdata (and
skb->dev == sdata->dev) with neither a reference nor any locking:

 - the lists have no lock: the softirq producer list_add_tail()s while the
   mac_wq worker list_del()s, so sibling interfaces on the same phy corrupt
   the list;

 - the workers dereference the interface after it may have been freed.
   mac802154_rx_mac_cmd_worker() touches mac_pkt->sdata directly, and
   mac802154_rx_beacon_worker() -> mac802154_process_beacon() dereferences
   skb->dev (== sdata->dev). Removing an interface frees its sdata
   (netdev_priv) while a queued frame still points at it, so a later worker
   run is a use-after-free.

Reproduced under KASAN by flooding a victim interface with MAC command
frames and removing it (the beacon path is the same class via skb->dev):

  BUG: KASAN: slab-use-after-free in mac802154_rx_mac_cmd_worker+0x463/0x630 [mac802154]
  Read of size 4 at addr ffff888002f9ea18 by task kworker/u8:1/31
  Workqueue: phy0-mac-cmds mac802154_rx_mac_cmd_worker [mac802154]
  Call Trace:
   mac802154_rx_mac_cmd_worker+0x463/0x630 [mac802154]
   process_one_work+0x611/0xe80
   worker_thread+0x52e/0xdc0
   kthread+0x30c/0x630
   ret_from_fork+0x2fd/0x3e0

Fix both lists together:

 - add local->rx_lock and take it around every list access: the softirq
   producer (plain spin_lock, softirq context) and the workers and flush
   (spin_lock_bh, process context);

 - pin the interface for the lifetime of a queued frame with
   netdev_hold()/netdev_put(), so the worker can safely dereference sdata /
   skb->dev even while the interface is being removed;

 - dequeue under the lock at the head and loop-drain the whole list in the
   workers (they previously processed one frame per run and relied on a
   later enqueue to drain the rest);

 - drop not-yet-started frames of an interface before it is unregistered,
   from ieee802154_if_remove() (after the RCU grace period) and from the
   ieee802154_remove_interfaces() loop -- the latter is the whole-phy
   teardown path, which does not go through ieee802154_if_remove().

An in-flight worker that already dequeued a frame keeps its own netdev
reference; unregister_netdevice() then waits it out in netdev_run_todo(),
which runs at rtnl_unlock() (rtnl released) and after the interface has
been closed, so it does not pin rtnl. A worker blocked in an association
TX only delays that one interface's unregister (the usual "waiting for %s
to become free"), it does not hold rtnl. netdev_hold() is used for this
reason instead of a cancel_work_sync() under rtnl, which would block on
the worker's unbounded MLME TX wait via ieee802154_sync_queue().

The mac-command worker additionally skips processing for a stopped
interface (ieee802154_sdata_running()), avoiding a needless association
response during teardown.

Fixes: 57588c71177f ("mac802154: Handle passive scanning")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v3:
 - Cover rx_beacon_list too: its worker dereferences the interface via
   skb->dev in mac802154_process_beacon(), so it has the same UAF. v2
   wrongly stated the beacon worker never touches the interface
   (raised by Miquel Raynal and by automated race analysis on the list).
 - Use per-frame netdev_hold() instead of drain + cancel_work_sync(). The
   cancel_work_sync() ran under rtnl in ieee802154_if_remove() and could
   pin rtnl for the length of an in-flight association TX
   (ieee802154_sync_queue(), unbounded). The netdev reference is instead
   waited on in netdev_run_todo(), after rtnl is dropped (raised by
   Jakub Kicinski and by automated race analysis).
 - Fix whole-phy teardown: v2 drained in ieee802154_unregister_hw() before
   the interfaces were stopped, so a frame could still be queued after the
   drain and run against freed sdata. Drain per-interface in the
   ieee802154_remove_interfaces() loop instead.
 - Loop-drain the workers (also fixes a pre-existing one-frame-per-run
   stranding); required so queued netdev references cannot be left pending.
 - Include the KASAN splat.

v2: https://lore.kernel.org/all/20260722101608.37744-1-security@auditcode.ai/
v1: https://lore.kernel.org/all/20260710111353.12138-1-security@auditcode.ai/

 include/net/cfg802154.h      |   1 +
 net/mac802154/ieee802154_i.h |   8 +++
 net/mac802154/iface.c        |   6 ++
 net/mac802154/main.c         |   1 +
 net/mac802154/rx.c           | 120 ++++++++++++++++++++++++++++-------
 net/mac802154/scan.c         |  10 +--
 6 files changed, 117 insertions(+), 29 deletions(-)

diff --git a/include/net/cfg802154.h b/include/net/cfg802154.h
index 76d2cd2e2b30..2e960441ea49 100644
--- a/include/net/cfg802154.h
+++ b/include/net/cfg802154.h
@@ -376,6 +376,7 @@ struct cfg802154_mac_pkt {
 	struct list_head node;
 	struct sk_buff *skb;
 	struct ieee802154_sub_if_data *sdata;
+	netdevice_tracker dev_tracker;
 	u8 page;
 	u8 channel;
 };
diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 8f2bff268392..279ff4e9b11e 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;
+	/* Serializes rx_beacon_list and rx_mac_cmd_list against the RX
+	 * softirq producer, the mac_wq workers and the teardown flush.
+	 */
+	spinlock_t rx_lock;
 
 	/* Association */
 	struct ieee802154_pan_device *assoc_dev;
@@ -300,6 +304,10 @@ static inline bool mac802154_is_beaconing(struct ieee802154_local *local)
 }
 
 void mac802154_rx_mac_cmd_worker(struct work_struct *work);
+void mac802154_flush_list(struct list_head *list,
+			  struct ieee802154_sub_if_data *sdata);
+void mac802154_flush_queued_pkts(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..31353795fa24 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -694,6 +694,7 @@ void ieee802154_if_remove(struct ieee802154_sub_if_data *sdata)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	synchronize_rcu();
+	mac802154_flush_queued_pkts(sdata->local, sdata);
 	unregister_netdevice(sdata->dev);
 }
 
@@ -705,6 +706,11 @@ void ieee802154_remove_interfaces(struct ieee802154_local *local)
 	list_for_each_entry_safe(sdata, tmp, &local->interfaces, list) {
 		list_del_rcu(&sdata->list);
 
+		/* Best-effort: a frame the RX softirq queues for this sdata
+		 * after the flush still pins the netdev, so the
+		 * unregister_netdevice() below waits it out.
+		 */
+		mac802154_flush_queued_pkts(local, sdata);
 		unregister_netdevice(sdata->dev);
 	}
 	mutex_unlock(&local->iflist_mtx);
diff --git a/net/mac802154/main.c b/net/mac802154/main.c
index ea1efef3572a..386c086c79d1 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_lock);
 	mutex_init(&local->iflist_mtx);
 
 	tasklet_setup(&local->tasklet, ieee802154_tasklet_handler);
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index cd8f2a11920d..19b5382e85a8 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -35,16 +35,23 @@ void mac802154_rx_beacon_worker(struct work_struct *work)
 		container_of(work, struct ieee802154_local, rx_beacon_work);
 	struct cfg802154_mac_pkt *mac_pkt;
 
-	mac_pkt = list_first_entry_or_null(&local->rx_beacon_list,
-					   struct cfg802154_mac_pkt, node);
-	if (!mac_pkt)
-		return;
+	for (;;) {
+		spin_lock_bh(&local->rx_lock);
+		mac_pkt = list_first_entry_or_null(&local->rx_beacon_list,
+						   struct cfg802154_mac_pkt, node);
+		if (mac_pkt)
+			list_del(&mac_pkt->node);
+		spin_unlock_bh(&local->rx_lock);
+		if (!mac_pkt)
+			break;
 
-	mac802154_process_beacon(local, mac_pkt->skb, mac_pkt->page, mac_pkt->channel);
+		mac802154_process_beacon(local, mac_pkt->skb,
+					 mac_pkt->page, mac_pkt->channel);
 
-	list_del(&mac_pkt->node);
-	kfree_skb(mac_pkt->skb);
-	kfree(mac_pkt);
+		netdev_put(mac_pkt->sdata->dev, &mac_pkt->dev_tracker);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
 }
 
 static bool mac802154_should_answer_beacon_req(struct ieee802154_local *local)
@@ -68,22 +75,15 @@ static bool mac802154_should_answer_beacon_req(struct ieee802154_local *local)
 	return interval == IEEE802154_ACTIVE_SCAN_DURATION;
 }
 
-void mac802154_rx_mac_cmd_worker(struct work_struct *work)
+static void mac802154_rx_mac_cmd(struct ieee802154_local *local,
+				 struct cfg802154_mac_pkt *mac_pkt)
 {
-	struct ieee802154_local *local =
-		container_of(work, struct ieee802154_local, rx_mac_cmd_work);
-	struct cfg802154_mac_pkt *mac_pkt;
 	u8 mac_cmd;
 	int rc;
 
-	mac_pkt = list_first_entry_or_null(&local->rx_mac_cmd_list,
-					   struct cfg802154_mac_pkt, node);
-	if (!mac_pkt)
-		return;
-
 	rc = ieee802154_get_mac_cmd(mac_pkt->skb, &mac_cmd);
 	if (rc)
-		goto out;
+		return;
 
 	switch (mac_cmd) {
 	case IEEE802154_CMD_BEACON_REQ:
@@ -121,11 +121,81 @@ void mac802154_rx_mac_cmd_worker(struct work_struct *work)
 	default:
 		break;
 	}
+}
 
-out:
-	list_del(&mac_pkt->node);
-	kfree_skb(mac_pkt->skb);
-	kfree(mac_pkt);
+void mac802154_rx_mac_cmd_worker(struct work_struct *work)
+{
+	struct ieee802154_local *local =
+		container_of(work, struct ieee802154_local, rx_mac_cmd_work);
+	struct cfg802154_mac_pkt *mac_pkt;
+
+	for (;;) {
+		spin_lock_bh(&local->rx_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_lock);
+		if (!mac_pkt)
+			break;
+
+		/* A stopped interface cannot transmit; skipping avoids a
+		 * needless association response (and the !netif_running()
+		 * warning it would trip) during teardown. The beacon worker
+		 * needs no such check as it never transmits.
+		 */
+		if (ieee802154_sdata_running(mac_pkt->sdata))
+			mac802154_rx_mac_cmd(local, mac_pkt);
+
+		netdev_put(mac_pkt->sdata->dev, &mac_pkt->dev_tracker);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
+}
+
+/**
+ * mac802154_flush_list - free queued RX frames on @list
+ * @list: rx_beacon_list or rx_mac_cmd_list
+ * @sdata: only free frames received on this interface, or %NULL for all
+ *
+ * Each frame pins the net_device it was received on (via netdev_hold()),
+ * so release that reference as the frame is dropped. Caller must hold
+ * local->rx_lock.
+ */
+void mac802154_flush_list(struct list_head *list,
+			  struct ieee802154_sub_if_data *sdata)
+{
+	struct cfg802154_mac_pkt *mac_pkt, *tmp;
+
+	list_for_each_entry_safe(mac_pkt, tmp, list, node) {
+		if (sdata && mac_pkt->sdata != sdata)
+			continue;
+		list_del(&mac_pkt->node);
+		netdev_put(mac_pkt->sdata->dev, &mac_pkt->dev_tracker);
+		kfree_skb(mac_pkt->skb);
+		kfree(mac_pkt);
+	}
+}
+
+/**
+ * mac802154_flush_queued_pkts - drop queued RX work referencing @sdata
+ * @local: the mac802154 device
+ * @sdata: interface being removed
+ *
+ * The workers dereference the queued frame's interface directly
+ * (mac_pkt->sdata) or through skb->dev in mac802154_process_beacon(). Drop
+ * the not-yet-started entries belonging to @sdata before it is unregistered
+ * so their netdev reference is released; an entry already dequeued by a
+ * running worker keeps its own reference until the worker completes, which
+ * unregister_netdevice() then waits out.
+ */
+void mac802154_flush_queued_pkts(struct ieee802154_local *local,
+				 struct ieee802154_sub_if_data *sdata)
+{
+	spin_lock_bh(&local->rx_lock);
+	mac802154_flush_list(&local->rx_beacon_list, sdata);
+	mac802154_flush_list(&local->rx_mac_cmd_list, sdata);
+	spin_unlock_bh(&local->rx_lock);
 }
 
 static int
@@ -221,7 +291,10 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		mac_pkt->sdata = sdata;
 		mac_pkt->page = sdata->local->scan_page;
 		mac_pkt->channel = sdata->local->scan_channel;
+		netdev_hold(sdata->dev, &mac_pkt->dev_tracker, GFP_ATOMIC);
+		spin_lock(&sdata->local->rx_lock);
 		list_add_tail(&mac_pkt->node, &sdata->local->rx_beacon_list);
+		spin_unlock(&sdata->local->rx_lock);
 		queue_work(sdata->local->mac_wq, &sdata->local->rx_beacon_work);
 		return NET_RX_SUCCESS;
 
@@ -233,7 +306,10 @@ ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 
 		mac_pkt->skb = skb_get(skb);
 		mac_pkt->sdata = sdata;
+		netdev_hold(sdata->dev, &mac_pkt->dev_tracker, GFP_ATOMIC);
+		spin_lock(&sdata->local->rx_lock);
 		list_add_tail(&mac_pkt->node, &sdata->local->rx_mac_cmd_list);
+		spin_unlock(&sdata->local->rx_lock);
 		queue_work(sdata->local->mac_wq, &sdata->local->rx_mac_cmd_work);
 		return NET_RX_SUCCESS;
 
diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index 300d4584533e..245e32970c05 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -104,13 +104,9 @@ static unsigned int mac802154_scan_get_channel_time(u8 duration_order,
 
 static void mac802154_flush_queued_beacons(struct ieee802154_local *local)
 {
-	struct cfg802154_mac_pkt *mac_pkt, *tmp;
-
-	list_for_each_entry_safe(mac_pkt, tmp, &local->rx_beacon_list, node) {
-		list_del(&mac_pkt->node);
-		kfree_skb(mac_pkt->skb);
-		kfree(mac_pkt);
-	}
+	spin_lock_bh(&local->rx_lock);
+	mac802154_flush_list(&local->rx_beacon_list, NULL);
+	spin_unlock_bh(&local->rx_lock);
 }
 
 static void
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2026-07-25 13:52 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-25 13:51 [PATCH wpan v3] mac802154: fix use-after-free of sdata via queued RX frames Ibrahim Hashimov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox