All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] mac802154: flush rx_mac_cmd_list before freeing sdata
@ 2026-07-10 11:13 Ibrahim Hashimov
  0 siblings, 0 replies; only message in thread
From: Ibrahim Hashimov @ 2026-07-10 11:13 UTC (permalink / raw)
  To: Alexander Aring, Stefan Schmidt, Miquel Raynal, David S . Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, linux-wpan, netdev, linux-kernel, stable

mac802154_rx_mac_cmd_worker() (net/mac802154/rx.c) is queued on
local->mac_wq every time a MAC-command frame (assoc req/resp, disassoc
notify, beacon req) is received on any interface of a given phy. Each
queued struct cfg802154_mac_pkt stashes a *raw* pointer to the
receiving interface's ieee802154_sub_if_data (sdata) in
ieee802154_subif_frame():

	mac_pkt->sdata = sdata;
	list_add_tail(&mac_pkt->node, &sdata->local->rx_mac_cmd_list);
	queue_work(sdata->local->mac_wq, &sdata->local->rx_mac_cmd_work);

and the worker later dereferences it with no liveness check at all,
e.g. for IEEE802154_CMD_ASSOCIATION_REQ:

	if (mac_pkt->sdata->wpan_dev.iftype != NL802154_IFTYPE_COORD)

Neither teardown path drains this queue before the sdata it points to
is freed:

 * ieee802154_if_remove() (net/mac802154/iface.c), reached from the
   nl802154 NL802154_CMD_DEL_INTERFACE handler, does list_del_rcu() +
   synchronize_rcu() + unregister_netdevice(sdata->dev) -- which frees
   sdata via priv_destructor/needs_free_netdev -- without touching
   local->mac_wq or local->rx_mac_cmd_list at all.

 * ieee802154_unregister_hw() (net/mac802154/main.c) only flushes
   local->workqueue (the DATA-path queue) before calling
   ieee802154_remove_interfaces(), which frees every sdata on the
   phy; local->mac_wq is drained only via destroy_workqueue() much
   later, after the interfaces (and their sdata) are already gone.

Either way, if mac802154_rx_mac_cmd_worker() is already queued (or
races back in from a frame received just before teardown), it runs
after the free and dereferences freed memory -- confirmed under
KASAN: flooding a victim NODE interface with MAC_CMD frames from a
MONITOR interface on a sibling phy, then deleting the victim via
NL802154_CMD_DEL_INTERFACE, reliably produces:

  BUG: KASAN: use-after-free in mac802154_rx_mac_cmd_worker+0x463/0x630 [mac802154]
  Read of size 4 at addr ffff888004b22a18 by task kworker/u8:1/31
  Workqueue: phy0-mac-cmds mac802154_rx_mac_cmd_worker [mac802154]
  Freed by task 498: ... (the DEL_INTERFACE task, ieee802154_if_remove)

Verified on the same v6.19 KASAN stand with this patch applied: the
identical MONITOR-flood-then-DEL_INTERFACE reproducer no longer trips
the use-after-free report in mac802154_rx_mac_cmd_worker().

Fix it the same way mac802154_flush_queued_beacons() (net/mac802154/scan.c)
already flushes local->rx_beacon_list on scan cleanup, plus a
cancel_work_sync() step: rx_beacon_work's worker never dereferences
sdata, so the existing sibling doesn't need it, but rx_mac_cmd_work's
does. Add mac802154_flush_queued_mac_cmds(local, sdata):

 - cancel_work_sync(&local->rx_mac_cmd_work) waits out a run already
   in flight (still safe -- nothing has been freed yet) and blocks any
   new run from starting while we hold the RTNL;
 - every rx_mac_cmd_list entry whose ->sdata matches (or every entry,
   when called with sdata == NULL for full-teardown) is then dropped,
   so no future run of the worker can see it;
 - if entries belonging to *other*, still-live interfaces on the same
   local remain, the work is re-queued so they still get processed.

Call it from both teardown paths:

 - ieee802154_if_remove(), before unregister_netdevice(sdata->dev),
   filtered to the sdata being removed (other interfaces on the same
   phy may have legitimate entries in flight);
 - ieee802154_unregister_hw(), before ieee802154_remove_interfaces(),
   with sdata == NULL since every interface on the local is going
   away and local->workqueue's flush_workqueue() does not cover
   local->mac_wq.

This mirrors how mac80211 drains per-interface work (e.g. the
analogous per-sdata work items cancelled from ieee80211_do_stop()
before an interface is torn down) and the existing mac802154 scan.c
list-flush idiom, applied to the one rx_mac_cmd_list consumer that
actually dereferences the freed interface.

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
---
 net/mac802154/ieee802154_i.h |  2 ++
 net/mac802154/iface.c        | 10 +++++++++
 net/mac802154/main.c         | 11 ++++++++++
 net/mac802154/rx.c           | 42 ++++++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+)

diff --git a/net/mac802154/ieee802154_i.h b/net/mac802154/ieee802154_i.h
index 8f2bff268392..e3c5c8d5b5d0 100644
--- a/net/mac802154/ieee802154_i.h
+++ b/net/mac802154/ieee802154_i.h
@@ -300,6 +300,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 000be60d9580..a5aa213e25a2 100644
--- a/net/mac802154/iface.c
+++ b/net/mac802154/iface.c
@@ -694,6 +694,16 @@ void ieee802154_if_remove(struct ieee802154_sub_if_data *sdata)
 	mutex_unlock(&sdata->local->iflist_mtx);
 
 	synchronize_rcu();
+
+	/*
+	 * Drop any rx_mac_cmd_list entry still pointing at this sdata
+	 * before it is freed below: mac802154_rx_mac_cmd_worker() runs
+	 * asynchronously on local->mac_wq and derefs mac_pkt->sdata with
+	 * no liveness check of its own (see mac802154_flush_queued_mac_cmds()
+	 * for details).
+	 */
+	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..2f8c57e78db1 100644
--- a/net/mac802154/main.c
+++ b/net/mac802154/main.c
@@ -277,6 +277,17 @@ void ieee802154_unregister_hw(struct ieee802154_hw *hw)
 	tasklet_kill(&local->tasklet);
 	flush_workqueue(local->workqueue);
 
+	/*
+	 * tasklet_kill() above stops any further frame reaching
+	 * ieee802154_subif_frame(), but mac802154_rx_mac_cmd_worker() may
+	 * still be queued/running on local->mac_wq and derefs the sdata of
+	 * every interface ieee802154_remove_interfaces() is about to free
+	 * below. flush_workqueue(local->workqueue) does not cover it --
+	 * that is the DATA workqueue, not local->mac_wq -- so drain it
+	 * explicitly first.
+	 */
+	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..0b167f76cb23 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -128,6 +128,48 @@ out:
 	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
+ *
+ * Every queued &struct cfg802154_mac_pkt stashes a raw pointer to the
+ * interface it was received on (see ieee802154_subif_frame() below) which
+ * mac802154_rx_mac_cmd_worker() dereferences without ever checking whether
+ * that interface is still alive. Callers must invoke this before freeing
+ * @sdata -- or every interface on @local, when @sdata is %NULL -- so the
+ * worker can never run against freed memory:
+ *
+ *  - cancel_work_sync() waits out a run already in flight. That is still
+ *    safe to let finish because nothing has been freed yet, and it blocks
+ *    any new run from starting for as long as we hold the RTNL.
+ *  - every list entry pointing at @sdata (all of them, if @sdata is NULL)
+ *    is then dropped so no future run of the worker can see it.
+ *
+ * Mirrors mac802154_flush_queued_beacons() in scan.c, which does not need
+ * the cancel_work_sync() step because its worker never dereferences sdata.
+ */
+void mac802154_flush_queued_mac_cmds(struct ieee802154_local *local,
+				     struct ieee802154_sub_if_data *sdata)
+{
+	struct cfg802154_mac_pkt *mac_pkt, *tmp;
+
+	cancel_work_sync(&local->rx_mac_cmd_work);
+
+	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);
+	}
+
+	/* Other interfaces on @local may still have entries pending. */
+	if (!list_empty(&local->rx_mac_cmd_list))
+		queue_work(local->mac_wq, &local->rx_mac_cmd_work);
+}
+
 static int
 ieee802154_subif_frame(struct ieee802154_sub_if_data *sdata,
 		       struct sk_buff *skb, const struct ieee802154_hdr *hdr)
-- 
2.50.1 (Apple Git-155)


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

only message in thread, other threads:[~2026-07-10 11:14 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 11:13 [PATCH net] mac802154: flush rx_mac_cmd_list before freeing sdata Ibrahim Hashimov

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.