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

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

See
https://sashiko.dev/#/patchset/[email protected]?part=1
---
 net/batman-adv/bridge_loop_avoidance.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
index cec11f12..cc9ed2fb 100644
--- a/net/batman-adv/bridge_loop_avoidance.c
+++ b/net/batman-adv/bridge_loop_avoidance.c
@@ -544,9 +544,10 @@ batadv_bla_get_backbone_gw(struct batadv_priv *bat_priv, const u8 *orig,
 		batadv_bla_send_announce(bat_priv, entry);
 
 		/* this will be decreased in the worker thread */
-		atomic_inc(&entry->request_sent);
-		atomic_set(&entry->wait_periods, BATADV_BLA_WAIT_PERIODS);
-		atomic_inc(&bat_priv->bla.num_requests);
+		if (atomic_xchg(&entry->request_sent, 1) == 0) {
+			atomic_set(&entry->wait_periods, BATADV_BLA_WAIT_PERIODS);
+			atomic_inc(&bat_priv->bla.num_requests);
+		}
 	}
 
 	return entry;
@@ -649,10 +650,8 @@ static void batadv_bla_send_request(struct batadv_bla_backbone_gw *backbone_gw)
 			      backbone_gw->vid, BATADV_CLAIM_TYPE_REQUEST);
 
 	/* no local broadcasts should be sent or received, for now. */
-	if (!atomic_read(&backbone_gw->request_sent)) {
+	if (atomic_xchg(&backbone_gw->request_sent, 1) == 0)
 		atomic_inc(&backbone_gw->bat_priv->bla.num_requests);
-		atomic_set(&backbone_gw->request_sent, 1);
-	}
 }
 
 /**
@@ -873,10 +872,8 @@ static bool batadv_handle_announce(struct batadv_priv *bat_priv, u8 *an_addr,
 		/* if we have sent a request and the crc was OK,
 		 * we can allow traffic again.
 		 */
-		if (atomic_read(&backbone_gw->request_sent)) {
+		if (atomic_xchg(&backbone_gw->request_sent, 0) != 0)
 			atomic_dec(&backbone_gw->bat_priv->bla.num_requests);
-			atomic_set(&backbone_gw->request_sent, 0);
-		}
 	}
 
 	batadv_backbone_gw_put(backbone_gw);
@@ -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);
 
 			batadv_bla_del_backbone_claims(backbone_gw);
@@ -1507,8 +1504,10 @@ static void batadv_bla_periodic_work(struct work_struct *work)
 			if (!atomic_dec_and_test(&backbone_gw->wait_periods))
 				continue;
 
+			if (atomic_xchg(&backbone_gw->request_sent, 0) == 0)
+				continue;
+
 			atomic_dec(&backbone_gw->bat_priv->bla.num_requests);
-			atomic_set(&backbone_gw->request_sent, 0);
 		}
 		rcu_read_unlock();
 	}

---
base-commit: 57c6af492c1948145db835bb3ea2980472558298
change-id: 20260512-bla-atomic-request_sent-feeb9f1a08c5

Best regards,
--  
Sven Eckelmann <[email protected]>