Re: [PATCH RFC batadv] batman-adv: bla: avoid double decrement of bla.num_requests

[Simon Wunderlich <sw@simonwunderlich.de>]

On Tuesday, May 12, 2026 9:28:52 AM Central European Summer Time Sven 
Eckelmann wrote:
> The bla.num_requests is increased when a no request_sent was in progress.
> And it is decremented in various places (announcemnt was received, backbone
> is purged, periodic work). But the check if the request_sent is actually
> set to a specific state and the atomic_dec/_inc are not safe because they
> are not atomic (TOCTOU) and multiple such code portions can run
> concurrently.
> 
> This can be avoided by atomically replacing the request_sent state while
> checking the previous value. Only if the previous value was not actually in
> the correct state then the atomic_dec/_inc can be performed on
> bla.num_requests.
> 
> Fixes: a9ce0dc43e2c ("batman-adv: add basic bridge loop avoidance code")
> Signed-off-by: Sven Eckelmann <sven@narfation.org>

Looks good to me. Please double check the first sentence of the commit message 
("... when a no request_sent was in progress").

> ---
> Please consider this as a note what might have to be done. This is
> completely untested.
> 
> Especially think about batadv_bla_purge_backbone_gw - the old code confused
> me a lot and the new one might therefore be wrong.

[...]

> @@ -1249,7 +1246,7 @@ static void batadv_bla_purge_backbone_gw(struct
> batadv_priv *bat_priv, int now)
> 
>  purge_now:
>  			/* don't wait for the pending request 
anymore */
> -			if (atomic_read(&backbone_gw-
>request_sent))
> +			if (atomic_xchg(&backbone_gw-
>request_sent, 0) != 0)
>  				atomic_dec(&bat_priv-
>bla.num_requests);

I think this change is correct. The idea was to supress broadcasts while 
requests are pending, because the claim database may be out of sync and loops 
may happen in this time otherwise. Multiple requests could be in flight to 
different backbone gateways, so this num_request counter was used to track the 
number of requests. If a backbone_gw is purged, the associated request is no 
longer interesting, and the counter can be decreased.

backbone gateways are removed on tear down or on timeout, so this is the right 
place to decrease the counter.

Your change is consistent with the rest of the patch to decrease it with a 
"real" atomic operation.

I've not tested this patch, but feel free to add:

Reviewed-by: Simon Wunderlich <sw@simonwunderlich.de>

Thank you,
      Simon