[PATCH net 1/9] batman-adv: fix stale receive device on merged fragments

[PATCH net 1/9] batman-adv: fix stale receive device on merged fragments

[Simon Wunderlich]

From: Zhiling Zou <zhilinz@nebusec.ai>

Fragment reassembly reuses the skb from the highest-numbered buffered
fragment as the merged packet. When that fragment was received on a hard
interface which is deleted before the chain completes, the merged skb can
re-enter the receive path with a stale skb->dev and skb_iif.

batadv_batman_skb_recv() passes such merged packets through the normal
receive handlers again. DAT and bridge loop avoidance both derive the ARP
header length from skb->dev, so they can dereference the freed net_device
before the packet reaches the local mesh interface.

Refresh the receive device metadata from the current receive device before
running the packet handlers. This keeps internally reinjected merged
fragments consistent with the normal receive path after hard interface
teardown.

Fixes: 610bfc6bc99b ("batman-adv: Receive fragmented packets and merge")
Cc: stable@vger.kernel.org
Reported-by: Vega <vega@nebusec.ai>
Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/batman-adv/main.c b/net/batman-adv/main.c
index 77597171d6376..d89d44706269b 100644
--- a/net/batman-adv/main.c
+++ b/net/batman-adv/main.c
@@ -492,6 +492,10 @@ int batadv_batman_skb_recv(struct sk_buff *skb, struct net_device *dev,
 	if (!skb)
 		goto err_put;
 
+	/* Merged fragments re-enter here with reused skb metadata. */
+	skb->dev = dev;
+	skb->skb_iif = dev->ifindex;
+
 	/* packet should hold at least type and version */
 	if (unlikely(!pskb_may_pull(skb, 2)))
 		goto err_free;
-- 
2.47.3

Re: [PATCH net 1/9] batman-adv: fix stale receive device on merged fragments

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=1

> --- Patch [1]: [PATCH net 1/9] batman-adv: fix stale receive device on merged fragments ---
[...]
> On the receive side, batadv_bla_rx() is called from batadv_interface_rx()
> only after skb->dev has already been replaced:
> 
> net/batman-adv/mesh-interface.c:batadv_interface_rx() {
> 	...
> 	/* skb->dev & skb->pkt_type are set here */
> 	skb->protocol = eth_type_trans(skb, mesh_iface);
> 	...
> 	if (batadv_bla_rx(bat_priv, skb, vid, packet_type))
> 	...
> }
> 
> and batadv_bla_rx() does not call batadv_bla_process_claim() at all.
> 
> That leaves batadv_arp_get_type() in
> net/batman-adv/distributed-arp-table.c as the only consumer that can see
> the stale receive device of a merged fragment:
> 
> 	if (unlikely(!pskb_may_pull(skb, hdr_size + ETH_HLEN +
> 				    arp_hdr_len(skb->dev))))
> 
> reached via batadv_dat_snoop_incoming_arp_request()/_reply() from
> batadv_recv_unicast_packet()/batadv_recv_bcast_packet(), and only when
> CONFIG_BATMAN_ADV_DAT is built and bat_priv->distributed_arp_table is
> enabled.
> 
> Since this carries Cc: stable, would it be worth restating the affected
> path as DAT only, and mentioning that reaching it depends on DAT being
> compiled in and enabled?

No

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net 1/9] batman-adv: fix stale receive device on merged fragments

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Sven Eckelmann <sven@narfation.org>:

On Fri, 21 Aug 2026 11:48:05 +0200 you wrote:
> From: Zhiling Zou <zhilinz@nebusec.ai>
> 
> Fragment reassembly reuses the skb from the highest-numbered buffered
> fragment as the merged packet. When that fragment was received on a hard
> interface which is deleted before the chain completes, the merged skb can
> re-enter the receive path with a stale skb->dev and skb_iif.
> 
> [...]

Here is the summary with links:
  - [net,1/9] batman-adv: fix stale receive device on merged fragments
    https://git.kernel.org/netdev/net/c/ad46c907d7d9
  - [net,2/9] batman-adv: bla: avoid CRC corruption due to parallel claim add
    https://git.kernel.org/netdev/net/c/08645ab95768
  - [net,3/9] batman-adv: bla: prevent CRC corruptions after claim flush
    https://git.kernel.org/netdev/net/c/89f3502ff687
  - [net,4/9] batman-adv: dat: avoid unaligned fault in IP extraction
    https://git.kernel.org/netdev/net/c/0121afa52cdb
  - [net,5/9] batman-adv: dat: atomically update mac addresses
    https://git.kernel.org/netdev/net/c/e6de568d3eda
  - [net,6/9] batman-adv: fix TX priority extraction for BATADV_FORW_MCAST
    https://git.kernel.org/netdev/net/c/7aedb59b8099
  - [net,7/9] batman-adv: mcast: ensure unshared skb for multicast packets
    https://git.kernel.org/netdev/net/c/82bf207f48eb
  - [net,8/9] batman-adv: mcast: linearize skbuff for packet generation
    https://git.kernel.org/netdev/net/c/6a30a59e2660
  - [net,9/9] batman-adv: bla: fix freeing of claims on meshif deletion
    https://git.kernel.org/netdev/net/c/8d128c932bce

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH net 2/9] batman-adv: bla: avoid CRC corruption due to parallel claim add

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_bla_add_claim() is used to add claims and modify the backbone of
claims for CLAIM frames from remote backbones and local packets. When it
handles a claim, it needs to either

* add the new claim's CRC to the backbone CRC
* remove the already existing claim's CRC from the old backbone and add it
  to the new backbone

But when the "new" claim code was running in parallel to the "change
backbone" code, it can happen that the CRC was invalid because the
backbone_gw of the claim was changed twice in the "new" claim code path:

* CPU0 creates the claim for gateway A and publishes it in the claim
  hash. The crc16 of the address has not yet been added to A's crc at
  this point.

* CPU1 processes a claim frame of gateway B for the same client, finds
  the just published claim, and performs the ownership change: it
  switches the pointer to B, removes the crc16 from A's crc - which
  never contained it - and adds it to B's crc.

* CPU0 continues behind the creation branch, unconditionally switches
  the pointer back to A without compensating B's crc (its remove_crc
  is false for the creation path), and finally adds the crc16 to A's
  crc

The CRC is then wrong for both:

* claim belongs to A: but CRC is not part of backbone A's CRC
* claim doesn't belong to B: CRC is still part of backbone B's CRC

This wrong CRC is never recomputated from the stored claims. For local
backbone claims, this can also not recovered using syncs.

To avoid this, split the functionality in clear separate parts:

* new claim which always adds claim CRC to the backbone CRC (but never
  changes the already set backbone_gw of the claim back)

* update of existing claim which automatically changes the backbone_gw
  entry and only updates both backbone CRCs when there was an actual change

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c | 63 ++++++++++++++++----------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index 94e074235e158..a3530cc90c959 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -695,12 +695,14 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
 	struct batadv_bla_backbone_gw *old_backbone_gw;
 	struct batadv_bla_claim search_claim;
 	struct batadv_bla_claim *claim;
-	bool remove_crc = false;
 	int hash_added;
+	u16 claim_crc;
+	bool changed;
 
 	ether_addr_copy(search_claim.addr, mac);
 	search_claim.vid = vid;
 	claim = batadv_claim_hash_find(bat_priv, &search_claim);
+	claim_crc = crc16(0, mac, ETH_ALEN);
 
 	/* create a new claim entry if it does not exist yet. */
 	if (!claim) {
@@ -732,43 +734,56 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
 			kfree(claim);
 			return;
 		}
+
+		spin_lock_bh(&backbone_gw->crc_lock);
+		backbone_gw->crc ^= claim_crc;
+		spin_unlock_bh(&backbone_gw->crc_lock);
+
+		WRITE_ONCE(backbone_gw->lasttime, jiffies);
+
+		batadv_claim_put(claim);
+		return;
+	}
+
+	WRITE_ONCE(claim->lasttime, jiffies);
+
+	/* replace backbone_gw atomically and adjust reference counters */
+	spin_lock_bh(&claim->backbone_lock);
+	if (claim->backbone_gw != backbone_gw) {
+		changed = true;
+
+		old_backbone_gw = claim->backbone_gw;
+		kref_get(&backbone_gw->refcount);
+		claim->backbone_gw = backbone_gw;
 	} else {
-		WRITE_ONCE(claim->lasttime, jiffies);
-		if (claim->backbone_gw == backbone_gw)
-			/* no need to register a new backbone */
-			goto claim_free_ref;
+		old_backbone_gw = NULL;
+		changed = false;
+	}
+	spin_unlock_bh(&claim->backbone_lock);
 
+	if (changed) {
 		batadv_dbg(BATADV_DBG_BLA, bat_priv,
 			   "%s(): changing ownership for %pM, vid %d to gw %pM\n",
 			   __func__, mac, batadv_print_vid(vid),
 			   backbone_gw->orig);
 
-		remove_crc = true;
-	}
+		/* add claim address to new backbone_gw */
+		spin_lock_bh(&backbone_gw->crc_lock);
+		backbone_gw->crc ^= claim_crc;
+		spin_unlock_bh(&backbone_gw->crc_lock);
 
-	/* replace backbone_gw atomically and adjust reference counters */
-	spin_lock_bh(&claim->backbone_lock);
-	old_backbone_gw = claim->backbone_gw;
-	kref_get(&backbone_gw->refcount);
-	claim->backbone_gw = backbone_gw;
-	spin_unlock_bh(&claim->backbone_lock);
+		WRITE_ONCE(backbone_gw->lasttime, jiffies);
+	}
 
-	if (remove_crc) {
+	if (old_backbone_gw) {
 		/* remove claim address from old backbone_gw */
 		spin_lock_bh(&old_backbone_gw->crc_lock);
-		old_backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
+		old_backbone_gw->crc ^= claim_crc;
 		spin_unlock_bh(&old_backbone_gw->crc_lock);
-	}
-
-	batadv_backbone_gw_put(old_backbone_gw);
 
-	/* add claim address to new backbone_gw */
-	spin_lock_bh(&backbone_gw->crc_lock);
-	backbone_gw->crc ^= crc16(0, claim->addr, ETH_ALEN);
-	spin_unlock_bh(&backbone_gw->crc_lock);
-	WRITE_ONCE(backbone_gw->lasttime, jiffies);
+		batadv_backbone_gw_put(old_backbone_gw);
+	}
 
-claim_free_ref:
 	batadv_claim_put(claim);
 }
 
-- 
2.47.3

Re: [PATCH net 2/9] batman-adv: bla: avoid CRC corruption due to parallel claim add

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2388 bytes --]

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=2

[...]
> > @@ -732,43 +734,56 @@ static void batadv_bla_add_claim(struct batadv_priv *bat_priv,
> >  			kfree(claim);
> >  			return;
> >  		}
> > +
> > +		spin_lock_bh(&backbone_gw->crc_lock);
> > +		backbone_gw->crc ^= claim_crc;
> > +		spin_unlock_bh(&backbone_gw->crc_lock);
> 
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't an issue introduced by this patch, but how do these relative XOR
> updates interact with the absolute CRC store that still exists in
> batadv_bla_del_backbone_claims() at this commit?

Please, look literally one patch further.
https://lore.kernel.org/r/20260821094813.201800-4-sw@simonwunderlich.de

[...]
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This is a pre-existing inconsistency rather than something this patch
> introduces, but now that the compare and the swap of claim->backbone_gw
> both happen under claim->backbone_lock here, should the two remaining
> readers be annotated too?
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_del_backbone_claims() {
> 	...
> 		hlist_for_each_entry_safe(claim, node_tmp,
> 					  head, hash_entry) {
> 			if (claim->backbone_gw != backbone_gw)
> 				continue;
> 	...
> }
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_answer_request() {
> 	...
> 			/* only own claims are interesting */
> 			if (claim->backbone_gw != backbone_gw)
> 				continue;
> 	...
> }
> 
> Both do a plain load, not even READ_ONCE(), while types.h documents the
> field as:
> 
> 	/** @backbone_lock: lock protecting backbone_gw pointer */
> 	spinlock_t backbone_lock;
> 
> At both sites the pointer is only compared and never dereferenced, and in
> batadv_bla_del_backbone_claims() a stale comparison still keeps the CRC
> parity correct because the deferred batadv_claim_release() XORs the address
> out of whichever backbone the claim points at. So this looks like a
> KCSAN-visible plain-access race with no functional fallout, but would a
> READ_ONCE() or a comment on those two readers be worth adding so the
> documented locking rule matches the code?

No about this patch and definitely not high priority. Still made a ticket: 
https://www.open-mesh.org/issues/465 for Simon

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH net 3/9] batman-adv: bla: prevent CRC corruptions after claim flush

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When batadv_bla_del_backbone_claims() tried to remove all claims of a
backbone, it sets the CRC to 0. It assumes that the it had the last
reference of the claims because batadv_claim_release() (which runs after
the last reference was released), is XORing the crc16 of the claim address
with the backbone CRC.

If there would be a parallel holder of any of these references, it could
happen that the backbone CRC is (0 ^ crc16(delayed_released_claim)). Which
is the wrong starting point for the new claims it may receive when the
remote answers the claim request from batadv_bla_send_request().

This reinitializations can be completely dropped to avoid this problem.
batadv_claim_release() will take care of fixing the backbone CRC.

Cc: stable@vger.kernel.org
Fixes: 23721387c409 ("batman-adv: add basic bridge loop avoidance code")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/bridge_loop_avoidance.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index a3530cc90c959..3194ccdbce607 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -325,11 +325,6 @@ batadv_bla_del_backbone_claims(struct batadv_bla_backbone_gw *backbone_gw)
 		}
 		spin_unlock_bh(list_lock);
 	}
-
-	/* all claims gone, initialize CRC */
-	spin_lock_bh(&backbone_gw->crc_lock);
-	backbone_gw->crc = BATADV_BLA_CRC_INIT;
-	spin_unlock_bh(&backbone_gw->crc_lock);
 }
 
 /**
-- 
2.47.3

Re: [PATCH net 3/9] batman-adv: bla: prevent CRC corruptions after claim flush

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 3629 bytes --]

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=3

[...]
> Discovered by: gpt-5-6-sol · Missed by: opus-5, sashiko-gemini · Confirmed by: opus-5
> This isn't a bug introduced by this patch, but since the patch touches
> exactly this function: is the ownership test in the loop just above the
> removed hunk sufficiently synchronized?
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_del_backbone_claims() {
> 	...
> 	spin_lock_bh(list_lock);
> 	hlist_for_each_entry_safe(claim, node_tmp, head, hash_entry) {
> 		if (claim->backbone_gw != backbone_gw)
> 			continue;
> 
> 		hlist_del_rcu(&claim->hash_entry);
> 		batadv_claim_put(claim);
> 	}
> 	spin_unlock_bh(list_lock);
> 	...
> }
> 
> claim->backbone_gw is read here with only the claim hash bucket list_lock
> held, but the writer in batadv_bla_add_claim() only takes
> claim->backbone_lock:
> 
> net/batman-adv/bridge_loop_avoidance.c:batadv_bla_add_claim() {
> 	...
> 	/* replace backbone_gw atomically and adjust reference counters */
> 	spin_lock_bh(&claim->backbone_lock);
> 	if (claim->backbone_gw != backbone_gw) {
> 		changed = true;
> 
> 		old_backbone_gw = claim->backbone_gw;
> 		kref_get(&backbone_gw->refcount);
> 		claim->backbone_gw = backbone_gw;
> 	} else {
> 	...
> }
> 
> The two paths can run at the same time:
> 
> CPU0 (workqueue)
> batadv_bla_periodic_work()
>   batadv_bla_purge_backbone_gw()
>     batadv_bla_del_backbone_claims()   /* reads claim->backbone_gw */
>     hlist_del_rcu(&backbone_gw->hash_entry);
> 
> CPU1 (rx/tx softirq)
> batadv_bla_rx() / batadv_bla_tx()
>   batadv_bla_process_claim()
>     batadv_handle_claim()
>       batadv_bla_add_claim()           /* writes claim->backbone_gw */
> 
> The same interleaving looks reachable purely from received frames, since
> batadv_handle_announce() calls batadv_bla_send_request() on a CRC mismatch,
> which also calls batadv_bla_del_backbone_claims().
> 
> If the purge observes the stale owner after a racing CLAIM frame has
> reassigned the claim to another backbone gateway, does it unlink a claim
> that is now legitimately owned by that other gateway, so the next mesh frame
> from that client MAC makes batadv_bla_rx() claim it locally and two backbone
> gateways forward broadcast/multicast into the bridged LAN until the
> claim/CRC request protocol resynchronizes?
> 
> In the opposite order, if the purge observes the newly written owner and
> skips a claim that was just reassigned to the gateway being purged, the
> claim survives pointing at a backbone gw that is removed from the backbone
> hash immediately afterwards. Since batadv_bla_purge_claims() only expires
> claims owned by our own address, can such an entry linger and make
> batadv_bla_tx() keep dropping multicast frames from that client MAC until a
> unicast mesh frame triggers a re-claim?
> 
> Refcounting and the CRC XOR accounting do stay balanced here, because
> batadv_claim_release() re-reads the owner under claim->backbone_lock, so
> this looks like a claim table state consistency issue rather than memory
> corruption. 

Minor problem, self healing and not about this patch. Not sure if Simon wants 
to fix it.


> The code is unchanged at the end of the series
> (b32d9323b268). Would taking claim->backbone_lock around the ownership
> test, or comparing against a value read under that lock, be appropriate?

No, this is wrong. You would then complain that this patch is having a TOCTOU 
problem. I will not fall for this trap.

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH net 4/9] batman-adv: dat: avoid unaligned fault in IP extraction

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

Independent of the alignment of the ARP packet in the SKB, either the
batadv_arp_ip_src or the batadv_arp_ip_dst will have an unaligned access
(on HW without native unaligned read support).

Use get_unaligned() to handle this properly on all architectures.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 5c3a0e553593 ("batman-adv: Distributed ARP Table - add ARP parsing functions")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 0d5a9cb0affeb..76fd23db926c7 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -316,7 +316,10 @@ static u8 *batadv_arp_hw_src(struct sk_buff *skb, int hdr_size)
  */
 static __be32 batadv_arp_ip_src(struct sk_buff *skb, int hdr_size)
 {
-	return *(__force __be32 *)(batadv_arp_hw_src(skb, hdr_size) + ETH_ALEN);
+	u8 *src = batadv_arp_hw_src(skb, hdr_size) + ETH_ALEN;
+	__be32 *ip = (__force __be32 *)src;
+
+	return get_unaligned(ip);
 }
 
 /**
@@ -341,8 +344,9 @@ static u8 *batadv_arp_hw_dst(struct sk_buff *skb, int hdr_size)
 static __be32 batadv_arp_ip_dst(struct sk_buff *skb, int hdr_size)
 {
 	u8 *dst = batadv_arp_hw_src(skb, hdr_size) + ETH_ALEN * 2 + 4;
+	__be32 *ip = (__force __be32 *)dst;
 
-	return *(__force __be32 *)dst;
+	return get_unaligned(ip);
 }
 
 /**
-- 
2.47.3

[PATCH net 5/9] batman-adv: dat: atomically update mac addresses

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When a MAC address is updated in batadv_dat_entry_add(), it is done using a
simple copy function. A parallel reader might only see parts of this
update. In worst case, the reader is transporting the half updated MAC
address over the network or is creating an ARP response using it -
poisoning the ARP cache.

atomic64_t can be used to store the 48 bit of a mac address. A reader will
then either see the old mac address or the new one - never a mixture of
both.

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 2f1dfbe18507 ("batman-adv: Distributed ARP Table - implement local storage")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/distributed-arp-table.c | 58 +++++++++++++++++---------
 net/batman-adv/types.h                 |  2 +-
 2 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
index 76fd23db926c7..011cfdc47fab4 100644
--- a/net/batman-adv/distributed-arp-table.c
+++ b/net/batman-adv/distributed-arp-table.c
@@ -444,18 +444,19 @@ batadv_dat_entry_hash_find(struct batadv_priv *bat_priv, __be32 ip,
 static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip,
 				 u8 *mac_addr, unsigned short vid)
 {
+	u64 u64_mac = ether_addr_to_u64(mac_addr);
 	struct batadv_dat_entry *dat_entry;
 	int hash_added;
 
 	dat_entry = batadv_dat_entry_hash_find(bat_priv, ip, vid);
 	/* if this entry is already known, just update it */
 	if (dat_entry) {
-		if (!batadv_compare_eth(dat_entry->mac_addr, mac_addr))
-			ether_addr_copy(dat_entry->mac_addr, mac_addr);
+		atomic64_set(&dat_entry->mac_addr, u64_mac);
+
 		dat_entry->last_update = jiffies;
 		batadv_dbg(BATADV_DBG_DAT, bat_priv,
 			   "Entry updated: %pI4 %pM (vid: %d)\n",
-			   &dat_entry->ip, dat_entry->mac_addr,
+			   &dat_entry->ip, mac_addr,
 			   batadv_print_vid(vid));
 		goto out;
 	}
@@ -466,7 +467,7 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip,
 
 	dat_entry->ip = ip;
 	dat_entry->vid = vid;
-	ether_addr_copy(dat_entry->mac_addr, mac_addr);
+	atomic64_set(&dat_entry->mac_addr, u64_mac);
 	dat_entry->last_update = jiffies;
 	kref_init(&dat_entry->refcount);
 
@@ -482,7 +483,7 @@ static void batadv_dat_entry_add(struct batadv_priv *bat_priv, __be32 ip,
 	}
 
 	batadv_dbg(BATADV_DBG_DAT, bat_priv, "New entry added: %pI4 %pM (vid: %d)\n",
-		   &dat_entry->ip, dat_entry->mac_addr, batadv_print_vid(vid));
+		   &dat_entry->ip, mac_addr, batadv_print_vid(vid));
 
 out:
 	batadv_dat_entry_put(dat_entry);
@@ -939,6 +940,8 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid,
 			    struct netlink_callback *cb,
 			    struct batadv_dat_entry *dat_entry)
 {
+	u8 mac[ETH_ALEN];
+	u64 u64_mac;
 	int msecs;
 	void *hdr;
 
@@ -951,11 +954,12 @@ batadv_dat_cache_dump_entry(struct sk_buff *msg, u32 portid,
 	genl_dump_check_consistent(cb, hdr);
 
 	msecs = jiffies_to_msecs(jiffies - dat_entry->last_update);
+	u64_mac = atomic64_read(&dat_entry->mac_addr);
+	u64_to_ether_addr(u64_mac, mac);
 
 	if (nla_put_in_addr(msg, BATADV_ATTR_DAT_CACHE_IP4ADDRESS,
 			    dat_entry->ip) ||
-	    nla_put(msg, BATADV_ATTR_DAT_CACHE_HWADDRESS, ETH_ALEN,
-		    dat_entry->mac_addr) ||
+	    nla_put(msg, BATADV_ATTR_DAT_CACHE_HWADDRESS, ETH_ALEN, mac) ||
 	    nla_put_u16(msg, BATADV_ATTR_DAT_CACHE_VID, dat_entry->vid) ||
 	    nla_put_u32(msg, BATADV_ATTR_LAST_SEEN_MSECS, msecs)) {
 		genlmsg_cancel(msg, hdr);
@@ -1235,10 +1239,12 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 	struct sk_buff *skb_new;
 	unsigned short vid;
 	bool ret = false;
+	u8 mac[ETH_ALEN];
 	int hdr_size = 0;
 	__be32 ip_dst;
 	__be32 ip_src;
 	u16 type = 0;
+	u64 u64_mac;
 	u8 *hw_src;
 
 	if (!READ_ONCE(bat_priv->distributed_arp_table))
@@ -1267,6 +1273,9 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 
 	dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_dst, vid);
 	if (dat_entry) {
+		u64_mac = atomic64_read(&dat_entry->mac_addr);
+		u64_to_ether_addr(u64_mac, mac);
+
 		/* If the ARP request is destined for a local client the local
 		 * client will answer itself. DAT would only generate a
 		 * duplicate packet.
@@ -1275,7 +1284,7 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 		 * additional DAT answer may trigger kernel warnings about
 		 * a packet coming from the wrong port.
 		 */
-		if (batadv_is_my_client(bat_priv, dat_entry->mac_addr, vid)) {
+		if (batadv_is_my_client(bat_priv, mac, vid)) {
 			ret = true;
 			goto out;
 		}
@@ -1285,18 +1294,16 @@ bool batadv_dat_snoop_outgoing_arp_request(struct batadv_priv *bat_priv,
 		 * the backbone gws belonging to our backbone has claimed the
 		 * destination.
 		 */
-		if (!batadv_bla_check_claim(bat_priv,
-					    dat_entry->mac_addr, vid)) {
+		if (!batadv_bla_check_claim(bat_priv, mac, vid)) {
 			batadv_dbg(BATADV_DBG_DAT, bat_priv,
 				   "Device %pM claimed by another backbone gw. Don't send ARP reply!",
-				   dat_entry->mac_addr);
+				   mac);
 			ret = true;
 			goto out;
 		}
 
 		skb_new = batadv_dat_arp_create_reply(bat_priv, ip_dst, ip_src,
-						      dat_entry->mac_addr,
-						      hw_src, vid);
+						      mac, hw_src, vid);
 		if (!skb_new)
 			goto out;
 
@@ -1340,8 +1347,10 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 	struct sk_buff *skb_new;
 	unsigned short vid;
 	bool ret = false;
+	u8 mac[ETH_ALEN];
 	__be32 ip_src;
 	__be32 ip_dst;
+	u64 u64_mac;
 	u8 *hw_src;
 	u16 type;
 	int err;
@@ -1371,8 +1380,11 @@ bool batadv_dat_snoop_incoming_arp_request(struct batadv_priv *bat_priv,
 	if (!dat_entry)
 		goto out;
 
+	u64_mac = atomic64_read(&dat_entry->mac_addr);
+	u64_to_ether_addr(u64_mac, mac);
+
 	skb_new = batadv_dat_arp_create_reply(bat_priv, ip_dst, ip_src,
-					      dat_entry->mac_addr, hw_src, vid);
+					      mac, hw_src, vid);
 	if (!skb_new)
 		goto out;
 
@@ -1464,8 +1476,10 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
 	struct batadv_dat_entry *dat_entry = NULL;
 	bool dropped = false;
 	unsigned short vid;
+	u8 mac[ETH_ALEN];
 	__be32 ip_src;
 	__be32 ip_dst;
+	u64 u64_mac;
 	u8 *hw_src;
 	u8 *hw_dst;
 	u16 type;
@@ -1497,11 +1511,17 @@ bool batadv_dat_snoop_incoming_arp_reply(struct batadv_priv *bat_priv,
 	 * this frame would lead to doubled receive of an ARP reply.
 	 */
 	dat_entry = batadv_dat_entry_hash_find(bat_priv, ip_src, vid);
-	if (dat_entry && batadv_compare_eth(hw_src, dat_entry->mac_addr)) {
-		batadv_dbg(BATADV_DBG_DAT, bat_priv, "Doubled ARP reply removed: ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]; dat_entry: %pM-%pI4\n",
-			   hw_src, &ip_src, hw_dst, &ip_dst,
-			   dat_entry->mac_addr,	&dat_entry->ip);
-		dropped = true;
+	if (dat_entry) {
+		u64_mac = atomic64_read(&dat_entry->mac_addr);
+		u64_to_ether_addr(u64_mac, mac);
+
+		if (batadv_compare_eth(hw_src, mac)) {
+			batadv_dbg(BATADV_DBG_DAT, bat_priv,
+				   "Doubled ARP reply removed: ARP MSG = [src: %pM-%pI4 dst: %pM-%pI4]; dat_entry: %pM-%pI4\n",
+				   hw_src, &ip_src, hw_dst, &ip_dst,
+				   mac, &dat_entry->ip);
+			dropped = true;
+		}
 	}
 
 	/* Update our internal cache with both the IP addresses the node got
diff --git a/net/batman-adv/types.h b/net/batman-adv/types.h
index 42b6315735123..9bdc5a3e799e2 100644
--- a/net/batman-adv/types.h
+++ b/net/batman-adv/types.h
@@ -2176,7 +2176,7 @@ struct batadv_dat_entry {
 	__be32 ip;
 
 	/** @mac_addr: the MAC address associated to the stored IPv4 */
-	u8 mac_addr[ETH_ALEN];
+	atomic64_t mac_addr;
 
 	/** @vid: the vlan ID associated to this entry */
 	unsigned short vid;
-- 
2.47.3

[PATCH net 6/9] batman-adv: fix TX priority extraction for BATADV_FORW_MCAST

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_mcast_forw_mode_by_count() pushs the skb->data for BATADV_FORW_MCAST
forwarding via batadv_mcast_forw_mcsend(). But the
batadv_skb_set_priority() expects the ethernet header directly before
(skb->data + offset). With the moved skb->data, just some random data would
be accessed to get the priority data.

Move the batadv_skb_set_priority() before the decision about the handling
multicast packets and potential header modifications.

Cc: stable@vger.kernel.org
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/mesh-interface.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/batman-adv/mesh-interface.c b/net/batman-adv/mesh-interface.c
index 8e55b61dd2a64..e202088cf1fd4 100644
--- a/net/batman-adv/mesh-interface.c
+++ b/net/batman-adv/mesh-interface.c
@@ -305,6 +305,8 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 	if (batadv_compare_eth(ethhdr->h_dest, ectp_addr))
 		goto dropped;
 
+	batadv_skb_set_priority(skb, 0);
+
 	gw_mode = READ_ONCE(bat_priv->gw.mode);
 	if (is_multicast_ether_addr(ethhdr->h_dest)) {
 		/* if gw mode is off, broadcast every packet */
@@ -338,6 +340,9 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 
 send:
 		if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
+			/* WARNING batadv_mcast_forw_mode might add more headers
+			 * in front of the skb. and might even reallocate the skb
+			 */
 			forw_mode = batadv_mcast_forw_mode(bat_priv, skb, vid,
 							   &mcast_is_routable);
 			switch (forw_mode) {
@@ -355,8 +360,6 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
 		}
 	}
 
-	batadv_skb_set_priority(skb, 0);
-
 	/* ethernet packet should be broadcasted */
 	if (do_bcast) {
 		primary_if = batadv_primary_if_get_selected(bat_priv);
-- 
2.47.3

Re: [PATCH net 6/9] batman-adv: fix TX priority extraction for BATADV_FORW_MCAST

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 3445 bytes --]

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=6

[...]
> > @@ -338,6 +340,9 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
> >  
> >  send:
> >  		if (do_bcast && !is_broadcast_ether_addr(ethhdr->h_dest)) {
> > +			/* WARNING batadv_mcast_forw_mode might add more headers
> > +			 * in front of the skb. and might even reallocate the skb
> > +			 */
> >  			forw_mode = batadv_mcast_forw_mode(bat_priv, skb, vid,
> >  							   &mcast_is_routable);
> 
> Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol
> This isn't a bug introduced by this patch, but would it make sense to put
> this warning into the kernel-doc of the callee instead of (or in addition
> to) the call site?
> 
> Today only batadv_mcast_forw_push() carries the contract in
> net/batman-adv/multicast_forw.c:
> 
>   * Warning: This function may reallocate the skb data buffer via
>   * batadv_mcast_forw_expand_head()/... Any pointer into the skb data (e.g.
>   * obtained from skb->data or eth_hdr()) before this call must be
>   * considered invalid afterwards and has to be reacquired.
> 
> The two callers 

What are the two callers? I only find batadv_mcast_forw_mode_by_count()

> in net/batman-adv/multicast.c do not mention it.
> batadv_mcast_forw_mode_by_count() only says:
> 
>   * For a multicast packet with multiple destination originators, checks which
>   * mode to use. For BATADV_FORW_MCAST it also encapsulates the packet with a
>   * complete batman-adv multicast header.
> 
> and the exported entry point batadv_mcast_forw_mode() (declared in
> net/batman-adv/multicast.h) documents only:
> 
>   * Return: The forwarding mode as enum batadv_forw_mode.
> 
> Since batadv_mcast_forw_mode() is the exported symbol, wouldn't a future
> caller reading its kernel-doc have no way to know that skb->data was pushed
> or that the buffer may have been reallocated?

Will not modify this patch for this request. Linus (or anyone else) can deal 
with it: https://www.open-mesh.org/issues/466

> 
> Related to that: batadv_interface_tx() does not re-acquire ethhdr after
> batadv_mcast_forw_mode() returns, even though the new comment warns about
> reallocation. The only later use, is_broadcast_ether_addr(ethhdr->h_dest),
> is evaluated before the call, 

The later use is before? This is crazy

> so nothing is dereferenced stale right now.
> Is it worth re-acquiring it anyway, or noting in the comment that ethhdr
> must not be used past this point?

Will not modify the patch for this.

> 
> >  			switch (forw_mode) {
> 
> [ ... ]
> 
> > @@ -355,8 +360,6 @@ static netdev_tx_t batadv_interface_tx(struct sk_buff *skb,
> >  		}
> >  	}
> >  
> > -	batadv_skb_set_priority(skb, 0);
> > -
> >  	/* ethernet packet should be broadcasted */
> >  	if (do_bcast) {
> >  		primary_if = batadv_primary_if_get_selected(bat_priv);
> 
> Discovered by: opus-5 · Missed by: gpt-5-6-sol, sashiko-gemini · Confirmed by: gpt-5-6-sol
> This is a pre-existing issue and not caused by this patch, but while the
> priority handling is being looked at: should the multicast forwarding path
> also set skb->priority?

This is a feature (or maybe minor bugfix) request. Will definitely not deal 
with it here. Documented it at https://www.open-mesh.org/issues/467

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH net 7/9] batman-adv: mcast: ensure unshared skb for multicast packets

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

When a packet is transmitted via a batman-adv interface and has already
enough room for the header then nothing will make sure that the skbuff is
unshared. But it is not allowed to modify a currently shared skbuff.

Always make sure that the pskb_expand_head() is not only called for a too
small header but also for shared skbuffs.

Cc: stable@vger.kernel.org
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/multicast_forw.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/batman-adv/multicast_forw.c b/net/batman-adv/multicast_forw.c
index bae2a81109767..3927d28dcbb89 100644
--- a/net/batman-adv/multicast_forw.c
+++ b/net/batman-adv/multicast_forw.c
@@ -1105,8 +1105,7 @@ static int batadv_mcast_forw_expand_head(struct batadv_priv *bat_priv,
 		return -EINVAL;
 	}
 
-	if (skb_headroom(skb) < hdr_size &&
-	    pskb_expand_head(skb, hdr_size, 0, GFP_ATOMIC) < 0)
+	if (skb_cow(skb, hdr_size) < 0)
 		return -ENOMEM;
 
 	return 0;
-- 
2.47.3

[PATCH net 8/9] batman-adv: mcast: linearize skbuff for packet generation

[Simon Wunderlich]

From: Sven Eckelmann <sven@narfation.org>

batadv_mcast_forw_packet() and batadv_mcast_forw_scrape() is not only
called (indirectly) by the unsharing+linearizing batadv_recv_mcast_packet()
handler. When it is called (indirectly) by batadv_mcast_forw_mcsend() then
it will be unshared but not linearized. The SKB_LINEAR_ASSERT() can
therefore cause a fatal BUG().

The linearization should happen during the expansion of the head because
the scrape function can be hit already during the initial
batadv_mcast_forw_mode() selection code:

* batadv_interface_tx
* batadv_mcast_forw_mode
* batadv_mcast_forw_mode_by_count()
* batadv_mcast_forw_push()
  -> calls batadv_mcast_forw_expand_head() before everything else
* batadv_mcast_forw_push_tvlvs()
* batadv_mcast_forw_push_dests()
* batadv_mcast_forw_push_adjust_padding()
* batadv_mcast_forw_scrape()

Cc: stable@vger.kernel.org
Reported-by: Sashiko <sashiko-bot@kernel.org>
Fixes: 90039133221e ("batman-adv: mcast: implement multicast packet generation")
Signed-off-by: Sven Eckelmann <sven@narfation.org>
Signed-off-by: Simon Wunderlich <sw@simonwunderlich.de>
---
 net/batman-adv/multicast_forw.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/batman-adv/multicast_forw.c b/net/batman-adv/multicast_forw.c
index 3927d28dcbb89..ac97cd8b81dc9 100644
--- a/net/batman-adv/multicast_forw.c
+++ b/net/batman-adv/multicast_forw.c
@@ -1108,6 +1108,10 @@ static int batadv_mcast_forw_expand_head(struct batadv_priv *bat_priv,
 	if (skb_cow(skb, hdr_size) < 0)
 		return -ENOMEM;
 
+	/* batadv_mcast_forw_scrape() + batadv_mcast_forw_packet() require linearized skb */
+	if (skb_linearize(skb) < 0)
+		return -ENOMEM;
+
 	return 0;
 }
 
-- 
2.47.3

Re: [PATCH net 8/9] batman-adv: mcast: linearize skbuff for packet generation

[Sven Eckelmann]

[-- Attachment #1: Type: text/plain, Size: 2080 bytes --]

https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260821094813.201800-1-sw%40simonwunderlich.de?part=8

[...]
> Discovered by: gpt-5-6-sol, opus-5 · Missed by: sashiko-gemini
> This isn't a bug introduced by this patch, but the new comment states a
> whole-skb requirement that the existing kernel-doc contradicts.
> 
> batadv_mcast_forw_scrape() documents only a prefix requirement:
> 
>  * Caller needs to ensure that the region from the skb data's start up
>  * to/including the to be removed bytes are linearized.
> 
> while its body asserts the entire skb is linear:
> 
> 	SKB_LINEAR_ASSERT(skb);
> 
> with, in include/linux/skbuff.h:
> 
> 	#define SKB_LINEAR_ASSERT(skb)  BUG_ON(skb_is_nonlinear(skb))
> 
> Similarly, batadv_mcast_forw_packet()'s kernel-doc lists the caller
> obligations (set network/transport header, free the skb) but does not
> mention linearity at all; only the in-body comment mentions it, and it
> also understates the assert:
> 
> 	/* (at least) TVLV part needs to be linearized */
> 	SKB_LINEAR_ASSERT(skb);
> 
> Should the kernel-doc of batadv_mcast_forw_scrape() and
> batadv_mcast_forw_packet() be updated to say the whole skb must be linear?
> 
> Along the same lines, the invariant this patch establishes is not recorded
> at either interface boundary: batadv_mcast_forw_push()'s kernel-doc does
> not mention that on success the skb is linear, and neither the
> batadv_mcast_forw_mcsend() kernel-doc nor its declaration in
> net/batman-adv/multicast.h mentions the precondition that
> batadv_mcast_forw_packet() enforces with BUG_ON().
> 
> No in-tree caller violates this today: batadv_recv_mcast_packet() unshares
> and linearizes, and the batadv_mcast_forw_mcsend() path is now covered by
> the skb_linearize() added here. Would documenting the requirement help
> avoid a future caller reintroducing the BUG()? 

Will definitely not deal with this documentation work this bugfix patch. Linus 
or anyone else interested can work on it: https://www.open-mesh.org/issues/468

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]