[PATCH net v3] mac802154: validate pinned interface before beacon transmission

[PATCH net v3] mac802154: validate pinned interface before beacon transmission

[Yifan Zhang]

mac802154_beacon_worker() selects a beacon request under RCU and pins
the corresponding netdev before dropping the read lock.  The transmit
helper used to read the request again, so a concurrent interface switch
could make it dereference an unpinned netdev.

Use the pinned wpan_dev throughout transmission.  Under RTNL, verify that
the request still belongs to that interface and that the interface is
running.  Build the beacon from the protected template while still
holding RTNL, then hand it to the locked MLME transmit helper.  Free the
skb only on errors before that handoff.

This closes the interface-switch use-after-free without transmitting
through a stopped interface or leaking the skb on the stopped-device
path.

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>
---
Changes in v3:
- revalidate the request and pinned interface under RTNL
- serialize beacon template access with request replacement
- free the skb on every failure before the transmit-path handoff

 net/mac802154/scan.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/net/mac802154/scan.c b/net/mac802154/scan.c
index d393b1f4e74e4..b3c8076b8057a 100644
--- a/net/mac802154/scan.c
+++ b/net/mac802154/scan.c
@@ -354,35 +354,31 @@ 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 ieee802154_sub_if_data *sdata =
+		IEEE802154_WPAN_DEV_TO_SUB_IF(wpan_dev);
 	struct sk_buff *skb;
 	int ret;
 
-	/* Update the sequence number */
-	local->beacon.mhr.seq = atomic_inc_return(&wpan_dev->bsn) & 0xFF;
-
 	skb = alloc_skb(IEEE802154_BEACON_SKB_SZ, GFP_KERNEL);
 	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;
+	rtnl_lock();
+	beacon_req = rtnl_dereference(local->beacon_req);
+	if (unlikely(!beacon_req || beacon_req->wpan_dev != wpan_dev ||
+		     !ieee802154_sdata_running(sdata) ||
+		     !netif_running(sdata->dev))) {
+		ret = -ECANCELED;
+		goto out_free;
 	}
 
-	sdata = IEEE802154_WPAN_DEV_TO_SUB_IF(beacon_req->wpan_dev);
+	/* Update the sequence number */
+	local->beacon.mhr.seq = atomic_inc_return(&wpan_dev->bsn) & 0xFF;
 	skb->dev = sdata->dev;
 
-	rcu_read_unlock();
-
 	ret = ieee802154_beacon_push(skb, &local->beacon);
-	if (ret) {
-		kfree_skb(skb);
-		return ret;
-	}
+	if (ret)
+		goto out_free;
 
 	/* Using the MLME transmission helper for sending beacons is a bit
 	 * overkill because we do not really care about the final outcome.
@@ -402,7 +398,16 @@ static int mac802154_transmit_beacon(struct ieee802154_local *local,
 	 * transmissions, wich anyway does not have a huge impact on the
 	 * performances given the current design of the stack.
 	 */
-	return ieee802154_mlme_tx(local, sdata, skb);
+	ret = ieee802154_mlme_tx_locked(local, sdata, skb);
+	rtnl_unlock();
+
+	return ret;
+
+out_free:
+	rtnl_unlock();
+	kfree_skb(skb);
+
+	return ret;
 }
 
 void mac802154_beacon_worker(struct work_struct *work)
-- 
2.39.5 (Apple Git-154)

Re: [PATCH net v3] mac802154: validate pinned interface before 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.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

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.