Project

General

Profile

Actions

Bug #481

open

tt: prevent split lock for add existence check and event queueing

Added by Sven Eckelmann 15 days ago. Updated 11 days ago.

Status:
In Progress
Priority:
Normal
Target version:
-
Start date:
10/02/2026
Due date:
% Done:

0%

Estimated time:

Description

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

> [Severity: High]
> Does this close only one ordering of the race with the re-add path?
> 
> The add side still changes the flags and queues the ADD in two separate
> steps. batadv_tt_local_add_existing() clears PENDING inside a flags_lock
> scoped_guard and returns. batadv_tt_local_add() then queues the ADD after
> that lock is dropped:
> 
>     if (added)
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
>     ...
>     if (batadv_tt_local_update_flags(bat_priv, tt_local, iif_is_wifi, mark))
>         batadv_tt_local_event(bat_priv, tt_local, BATADV_NO_FLAGS);
> 
> batadv_tt_local_set_pending() can run in that window from the RX path:
> 
> batadv_tt_global_add()->batadv_tt_global_purge_local()->
>   batadv_tt_local_remove()->batadv_tt_local_mark_removed()->
>     batadv_tt_local_set_pending()
> 
> Here is one interleaving, starting with DEL#1 already queued and PENDING
> set:
> 
>   CPU1 (TX)                         CPU0 (RX)
>   batadv_tt_local_add()
>     batadv_tt_local_add_existing()
>       flags &= ~PENDING;
>                                     batadv_tt_local_mark_removed()
>                                       batadv_tt_local_set_pending()
>                                         DEL|ROAM merged into DEL#1
>                                         flags |= PENDING;
>     batadv_tt_local_event(NO_FLAGS)
>       ADD cancels DEL#1
> 
> There is a similar case with no earlier DEL. batadv_tt_local_update_flags()
> returns true on CPU1, CPU0 queues a new DEL and sets PENDING, and then the
> ADD from CPU1 cancels that DEL through the DEL+ADD case in
> __batadv_tt_local_event().
> 
> Either way, the entry would be left PENDING with no DEL queued. At the
> next commit, batadv_tt_local_purge_pending_clients() would remove it
> locally, but neighbours would never receive a DEL.
> 
> Wouldn't that give the same CRC mismatch and full table request that the
> commit message describes? Would the ADD also need to be queued inside the
> flags_lock section that clears PENDING or updates the flags?
Actions

Also available in: Atom PDF