[PATCH net] mac802154: use pinned interface for beacon transmission

[PATCH net] mac802154: use pinned interface for beacon transmission

[Yifan Zhang]

>From 1e3faf146431b4c28bdf0923fa54754c1f8d8902 Mon Sep 17 00:00:00 2001
From: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
Date: Wed, 28 Oct 2026 17:18:59 +0800
Subject: [PATCH net] mac802154: use pinned interface for beacon transmission

mac802154_beacon_worker() looks up beacon_req under RCU and takes a
reference on the selected interface's netdev before dropping the RCU read
lock. It then passes the selected wpan_dev to
mac802154_transmit_beacon().

mac802154_transmit_beacon() ignores that argument when choosing sdata and
reads local->beacon_req again. If the beacon request switches to another
interface between the two reads, the worker pins the old interface while
the transmit path uses the new one. Removing the new interface can then
free its netdev before ieee802154_mlme_tx_locked() dereferences it,
causing a KASAN use-after-free.

Derive sdata from the wpan_dev already selected and pinned by the worker.
This also removes the redundant RCU lookup.

The reproducer hit the use-after-free on two separate v7.3 KASAN boots
before this change and completed without a KASAN report in three runs
afterward.

Fixes: 5f26a690e8ef ("mac802154: fix netdev use-after-free in beacon worker")
Cc: stable@vger.kernel.org
Signed-off-by: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
---
 net/mac802154/scan.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index d393b1f..9f0e45b 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -353,7 +353,6 @@ int mac802154_process_beacon(struct ieee802154_local *local,
 static int mac802154_transmit_beacon(struct ieee802154_local *local,
 				     struct wpan_dev *wpan_dev)
 {
-	struct cfg802154_beacon_request *beacon_req;
 	struct ieee802154_sub_if_data *sdata;
 	struct sk_buff *skb;
 	int ret;
@@ -365,19 +364,9 @@ static int mac802154_transmit_beacon(struct ieee802154_local *local,
 	if (!skb)
 		return -ENOBUFS;
 
-	rcu_read_lock();
-	beacon_req = rcu_dereference(local->beacon_req);
-	if (unlikely(!beacon_req)) {
-		rcu_read_unlock();
-		kfree_skb(skb);
-		return -EINVAL;
-	}
-
-	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(beacon_req->wpan_dev);
+	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev);
 	skb->dev = sdata->dev;
 
-	rcu_read_unlock();
-
 	ret = ieee802154_beacon_push(skb, &local->beacon);
 	if (ret) {
 		kfree_skb(skb);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH net] mac802154: use pinned interface for beacon transmission

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] mac802154: use pinned interface for beacon transmission

[Miquel Raynal]

Hi Yifan,

On 30/10/2026 at 17:36:14 +08, Yifan Zhang <yfzhang23@stu.pku.edu.cn> wrote:

>>From 1e3faf146431b4c28bdf0923fa54754c1f8d8902 Mon Sep 17 00:00:00 2001
> From: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
> Date: Wed, 28 Oct 2026 17:18:59 +0800
> Subject: [PATCH net] mac802154: use pinned interface for beacon
> transmission

These lines should be in the header, they appear in the body. Something
is wrong with the contribution. You can use a tool such as b4 to help
you with this setup.

> mac802154_beacon_worker() looks up beacon_req under RCU and takes a
> reference on the selected interface's netdev before dropping the RCU read
> lock. It then passes the selected wpan_dev to
> mac802154_transmit_beacon().
>
> mac802154_transmit_beacon() ignores that argument when choosing sdata and
> reads local->beacon_req again. If the beacon request switches to another
> interface between the two reads, the worker pins the old interface while
> the transmit path uses the new one. Removing the new interface can then
> free its netdev before ieee802154_mlme_tx_locked() dereferences it,
> causing a KASAN use-after-free.
>
> Derive sdata from the wpan_dev already selected and pinned by the worker.
> This also removes the redundant RCU lookup.
>
> The reproducer hit the use-after-free on two separate v7.3 KASAN boots
> before this change and completed without a KASAN report in three runs
> afterward.
>
> Fixes: 5f26a690e8ef ("mac802154: fix netdev use-after-free in beacon worker")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
> ---

With the commit fixed,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH net] mac802154: use pinned interface for beacon transmission

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 4 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 2 · Low: 1 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] mac802154: warning and skb leak in mac802154_transmit_beacon()
  when iface down
- [Medium] After this patch, the race the commit message targets ends in
  WARN_ON_ONCE(!netif_running(sdata->dev)) in…
- [Medium] The beacon skb leaks whenever ieee802154_mlme_tx_locked()
  rejects the pinned interface.
- [Low] The commit body contains a raw mbox/format-patch header block:
  '>>From 1e3faf146431b4c28bdf0923fa54754c1f8d8902 Mon Sep 17 00:00:00…

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but beacon_work
  can stay armed after mac_wq is destroyed and the PHY is freed.
- [Low] local->beacon is a per-PHY beacon template, and two paths access
  it with locksets that do not intersect.

--

mac802154: use pinned interface for beacon transmission

With this patch, mac802154_transmit_beacon() takes sdata from the wpan_dev
that mac802154_beacon_worker() already selected and pinned with
netdev_hold(). It no longer re-reads local->beacon_req under RCU. The goal
is to avoid a use-after-free when beacon_req switches to another interface
and that interface is removed before ieee802154_mlme_tx_locked() runs.

> mac802154: use pinned interface for beacon transmission
>
> >>From 1e3faf146431b4c28bdf0923fa54754c1f8d8902 Mon Sep 17 00:00:00 2001
> From: Yifan Zhang <yfzhang23@stu.pku.edu.cn>
> Date: Wed, 28 Oct 2026 17:18:59 +0800
> Subject: [PATCH net] mac802154: use pinned interface for beacon transmission

[Severity: Low]
This isn't a bug, but these format-patch header lines ended up in the
commit body. Because of the leading ">>From" line, git mailinfo won't
treat them as in-body headers.

If the patch is applied as is, the changelog keeps:

- a second copy of the subject, with the "[PATCH net]" prefix
- a format-patch SHA that matches no commit in the tree
- a Date that doesn't match the author date

Could the patch be resent with git send-email, or could these lines be
stripped when it is applied?

> diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
> index d393b1f4e74e4..9f0e45bf4bcb5 100644
> --- a/net/mac802154/scan.c
> +++ b/net/mac802154/scan.c

[ ... ]

> @@ -365,19 +364,9 @@ static int mac802154_transmit_beacon(struct ieee802154_local *local,
>  	if (!skb)
>  		return -ENOBUFS;
>  
> -	rcu_read_lock();
> -	beacon_req = rcu_dereference(local->beacon_req);
> -	if (unlikely(!beacon_req)) {
> -		rcu_read_unlock();
> -		kfree_skb(skb);
> -		return -EINVAL;
> -	}
> -
> -	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(beacon_req->wpan_dev);
> +	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev);

[Severity: High]
Can this turn the race from the commit message into a WARN_ON_ONCE()
and an skb leak?

mac802154_beacon_worker() pins sdata->dev with netdev_hold() and checks
ieee802154_sdata_running() under rcu_read_lock(). Nothing is held
between rcu_read_unlock() and the rtnl_lock() taken inside
ieee802154_mlme_tx(), so the pinned interface can be closed in that
window.

The removed re-read of local->beacon_req used to catch this. If
mac802154_slave_close() had stopped beaconing through
mac802154_stop_beacons_locked(), the skb was freed and -EINVAL
returned. If the request had moved, the new interface was used. Now
the pinned wpan_dev is used unconditionally.

For beacon_req to move to another interface, as the commit message
describes, the old interface must already be down.
ieee802154_check_concurrent_iface() does not let two coordinator
interfaces run on one PHY, and NL802154_CMD_SEND_BEACONS requires
NL802154_FLAG_CHECK_NETDEV_UP. In that scenario the worker always
reaches ieee802154_mlme_tx_locked() with !netif_running(sdata->dev)
and local->open_count > 0:

net/mac802154/tx.c:ieee802154_mlme_tx_locked() {
    ...
	if (!local->open_count)
		return -ENETDOWN;

	if (WARN_ON_ONCE(!netif_running(sdata->dev)))
		return -ENETDOWN;
    ...
}

The same state occurs when the coordinator is closed while a monitor
interface on the same PHY stays open. With panic_on_warn set, this
takes the machine down.

Both -ENETDOWN returns also exit before the skb reaches
ieee802154_tx(), and nothing frees it. Callers of ieee802154_mlme_tx()
don't free the skb on error, because ieee802154_tx() consumes it on
its own paths. mac802154_transmit_beacon() just returns the error.
So the beacon skb leaks on either path, including when no other
interface is open.

Should mac802154_transmit_beacon() re-check under RTNL that
local->beacon_req still points at wpan_dev and that the interface is
still running, and free the skb otherwise? Or should the -ENETDOWN
returns in ieee802154_mlme_tx_locked() free the skb themselves?

[Severity: Medium]
Can the race described in the commit message now end in the
WARN_ON_ONCE() in ieee802154_mlme_tx_locked()?

mac802154_beacon_worker() checks ieee802154_sdata_running() under RCU
only. It then allocates and builds the beacon, and takes RTNL only inside
ieee802154_mlme_tx().

mac802154_stop_beacons_locked() uses the non-sync cancel_delayed_work(),
so a worker that is already running keeps going.

For beacon_req to move from interface A to interface B, B has to be a
coordinator that is up. nl802154_send_beacons() requires
NL802154_IFTYPE_COORD and NL802154_FLAG_CHECK_NETDEV_UP.

ieee802154_check_concurrent_iface() and ieee802154_check_mac_settings()
allow only one such interface to run per PHY. So A must be down by then.

Before this patch, mac802154_transmit_beacon() re-read beacon_req and
transmitted on B. With this change, the pinned and downed A is always
passed in. If B, or any other interface on the PHY, is still open when the
worker gets RTNL, local->open_count is non-zero and this fires:

net/mac802154/tx.c:ieee802154_mlme_tx_locked() {
    ...
	if (WARN_ON_ONCE(!netif_running(sdata->dev)))
		return -ENETDOWN;
    ...
}

The same applies if A is closed and another interface is opened before
the worker wins rtnl_lock().

The commit message calls the removed lookup redundant. Wasn't it the only
place where the worker noticed that beaconing had moved?

Would it make sense to re-check after rtnl_lock() that
rcu_dereference_rtnl(local->beacon_req) is non-NULL, that its ->wpan_dev
still equals wpan_dev, and that ieee802154_sdata_running(sdata) still
holds, and otherwise free the skb and return?

[Severity: Medium]
Does this path now leak the beacon skb whenever
ieee802154_mlme_tx_locked() rejects the pinned interface?

Both -ENETDOWN returns in ieee802154_mlme_tx_locked() exit without passing
the skb to ieee802154_tx() and without freeing it:

	if (!local->open_count)
		return -ENETDOWN;

	if (WARN_ON_ONCE(!netif_running(sdata->dev)))
		return -ENETDOWN;

mac802154_transmit_beacon() passes that error straight back from
ieee802154_mlme_tx() and never calls kfree_skb().

The removed beacon_req check used to free the skb and return -EINVAL when
beaconing had stopped.

mac802154_slave_close() stops beacons with the non-sync cancel and then
decrements local->open_count. If A is closed this way before the worker
takes RTNL, the race from the commit message now always ends on one of
these -ENETDOWN paths. That happens whether or not the new interface has
already been removed.

Should the skb be freed on the -ENETDOWN paths? Alternatively, the beacon
path could re-check under RTNL and free the skb when the check fails.

>  	skb->dev = sdata->dev;
>  
> -	rcu_read_unlock();
> -
>  	ret = ieee802154_beacon_push(skb, &local->beacon);

[Severity: Low]
This is a pre-existing issue, but local->beacon is accessed here with no
lock in common with mac802154_send_beacons_locked().

mac802154_transmit_beacon() writes local->beacon.mhr.seq and serializes
the whole template in ieee802154_beacon_push() before ieee802154_mlme_tx()
takes RTNL. Meanwhile, mac802154_send_beacons_locked() rebuilds the
template under RTNL:

	memset(&local->beacon, 0, sizeof(local->beacon));
	local->beacon.mhr.fc.type = IEEE802154_FC_TYPE_BEACON;
	...

Because mac802154_stop_beacons_locked() uses cancel_delayed_work(), a
running worker can serialize a template that is only partly rebuilt.

After this patch, skb->dev and the BSN come from the pinned interface,
while the template may describe the new request's wpan_dev. Such a frame
is only sent if the pinned interface was re-opened. Otherwise
ieee802154_mlme_tx_locked() rejects it.

Should this data race be handled too?

>  	if (ret) {
>  		kfree_skb(skb);

[Severity: High]
This isn't a bug introduced by this patch, but can beacon_work still be
armed after mac_wq is destroyed and the PHY is freed?

mac802154_stop_beacons_locked() only calls cancel_delayed_work(). A worker
that already passed its beacon_req check keeps running. At the end of
mac802154_beacon_worker(), it re-arms from its stale snapshot before
dropping the netdev reference:

	if (interval < IEEE802154_ACTIVE_SCAN_DURATION)
		queue_delayed_work(local->mac_wq, &local->beacon_work,
				   local->beacon_interval);
	netdev_put(sdata->dev, &dev_tracker);

In ieee802154_unregister_hw(), ieee802154_remove_interfaces() runs under
RTNL. The worker therefore blocks in ieee802154_mlme_tx(), and
unregistration waits for the worker's netdev_put(), which comes after the
re-arm. Then:

ieee802154_unregister_hw()
    destroy_workqueue(local->mac_wq);

runs while the beacon_work timer is still pending. The destroy_workqueue()
comment says all delayed_work must be cancelled before calling it.
net/mac802154 has no cancel_delayed_work_sync() for beacon_work.

ieee802154_free_hw() then frees the wpan_phy that holds ieee802154_local
and the embedded timer. When the timer fires, delayed_work_timer_fn()
would touch freed memory and queue work onto a destroyed workqueue.

This could be hit while a coordinator is beaconing, by deleting a hwsim
radio, by rmmod, or by unplugging a USB transceiver.

Would it help to stop the producers and call cancel_delayed_work_sync()
on beacon_work (and scan_work) before destroy_workqueue(local->mac_wq)?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179335297474.14852.9229658182764909973%40stu.pku.edu.cn