Project

General

Profile

Actions

Bug #480

open

tt: prevent batadv_tt_local_remove_now concurrency problems with batadv_tt_local_commit_changes_nolock()

Added by Sven Eckelmann 5 days ago. Updated about 5 hours ago.

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

0%

Estimated time:

Description

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

> [Severity: Medium]
> This isn't a bug introduced by this patch, but the immediate purge
> branch has a similar ordering problem.
> 
> batadv_tt_local_mark_removed() checks BATADV_TT_CLIENT_NEW under
> list_lock, but drops the lock before batadv_tt_local_remove_now() acts
> on the result. batadv_tt_local_remove_now() then unlinks the entry first
> and queues the DEL afterwards:
> 
>     tt_removed_node = batadv_hash_remove(bat_priv->tt.local_hash,
>                          batadv_compare_tt_entry,
>                          batadv_choose_tt, common);
>     if (!tt_removed_node)
>         return;
> 
>     batadv_tt_local_event(bat_priv, tt_local_entry, BATADV_TT_CLIENT_DEL);
> 
> If CPU0 stalls between those two calls while CPU1 commits, can this
> happen?
> 
> CPU0                              CPU1
> batadv_tt_local_remove_now()
>   batadv_hash_remove()
>                                   batadv_tt_local_commit_changes_nolock()
>                                     batadv_tt_local_transition_new()
>                                     batadv_tt_local_update_crc()
>                                     /* CRC for n + 1 excludes E */
>                                     batadv_tt_tvlv_container_update()
>                                     /* queued ADD for E sent in n + 1 */
>   batadv_tt_local_event(..., DEL)
>   /* DEL only sent in n + 2 */
> 
> Neighbours apply the ADD. Their CRC for us then includes E while ours
> doesn't, so they request the full table. The comment above
> batadv_tt_local_remove_now() mentions a parallel context announcing the
> entry, but only the size accounting handles that case.
> 
> The DEL-after-unlink order comes from the earlier series commit
> "batman-adv: tt: only queue local del event after successful unlink".
> The baseline queued the DEL before the unlink and had a mirror race with
> the same CRC mismatch.
> 
> As with the PENDING branch, holding list_lock around both steps wouldn't
> close this. batadv_tt_local_update_crc() and
> batadv_tt_tvlv_container_update() never take list_lock.

Related issues 1 (1 open — 0 closed)

Related to batman-adv - Bug #475: tt: Fix current event queuing guarentees for batadv_tt_local_remove_now()Newbatman-adv developers10/02/2026

Actions
Actions #1

Updated by Sven Eckelmann 5 days ago

  • Related to Bug #475: tt: Fix current event queuing guarentees for batadv_tt_local_remove_now() added
Actions #2

Updated by Sven Eckelmann 5 days ago

  • Subject changed from tt: prevent batadv_hash_remove concurrency problems with batadv_tt_local_commit_changes_nolock() to tt: prevent batadv_tt_local_remove_now concurrency problems with batadv_tt_local_commit_changes_nolock()
Actions

Also available in: Atom PDF