Thread (17 messages) flat view 17 messages, 3 authors, 2026-08-22

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

From: Sven Eckelmann <sven@narfation.org>
Date: 2026-08-22 11:01:24
Also in: batman, stable

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

[...]
quoted
@@ -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 (local)

[...]
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

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help