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
- signature.asc [application/pgp-signature] 228 bytes