Open-Mesh » batman-adv

And the netdev AI complaint https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930094558.3723766-2-sw%40simonwunderlich.de

> batadv_hash_remove() takes and releases the bucket list_lock before it
> returns. batadv_tt_local_event() only takes tt.changes_list_lock after
> that. Nothing seems to serialize this gap against batadv_tt_local_add()
> called from batadv_interface_tx() for the same MAC and VID.
> 
> The remove side can be reached from a remote TT announcement:
> 
> batadv_tt_global_add()
>   batadv_tt_global_purge_local()
>     batadv_tt_local_remove()
>       batadv_tt_local_remove_now()
> 
> Consider entry E, which is still NEW and has a queued add event:
> 
> CPU0 (remove path)
> batadv_hash_remove(E)
>   /* E unlinked, its add event still queued */
> 
> CPU1 (batadv_tt_local_add())
> batadv_tt_local_hash_find()      /* misses E */
> batadv_tt_local_create()         /* new entry E' */
> batadv_hash_add(E')
> batadv_tt_local_event(E', BATADV_NO_FLAGS)
>   /* same addr/vid as E's pending add, so only flags are updated */
> 
> CPU0
> batadv_tt_local_event(E, BATADV_TT_CLIENT_DEL)
>   /* matches the remaining add by addr/vid only */
>   if (del_op_requested != del_op_entry) {
>       list_del(&entry->list);
>       kmem_cache_free(batadv_tt_change_cache, entry);
>       changes--;
> 
> E' is then in tt.local_hash with BATADV_TT_CLIENT_NEW set and no add
> event queued. The commit message says this should be prevented.
> 
> Later packets from the client do not queue it again, because
> batadv_tt_local_add_existing() returns false for a NEW entry that is not
> PENDING.
> 
> If local_changes drops to 0, batadv_tt_local_commit_changes_nolock()
> returns early and E' stays unannounced. A later unrelated commit would
> move E' into the CRC through batadv_tt_local_transition_new() without an
> add diff. Neighbours would then see a CRC mismatch and fall back to full
> table requests.
> 
> Before this patch the delete event was queued while E was still in the
> hash. An add that ran after the unlink would then get a fresh add event
> that nothing cancels.
> 
> The later patches in the series don't seem to change this part of
> batadv_tt_local_remove_now().
> 
> Could the delete event instead be queued inside the same bucket list_lock
> critical section as the unlink, and only when this exact object was
> found? batadv_tt_local_set_pending() in "batman-adv: tt: queue local DEL
> event under bucket lock" already takes list_lock and then
> changes_list_lock in that order.

The only option I have in mind is also the one mentioned in the last section. But this will need more changes because the locking is done inside batadv_hash_remove and there is no code to allow handling the lock selection outside of this (or a similar) function.

See #482#note-2