Project

General

Profile

Actions

Bug #478

open

tt: prevent __batadv_tt_local_event() from queuing del for already unhashed local entry

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

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

> [Severity: Medium]
> This is a pre-existing issue, but now that the bucket lock is taken here,
> should this also check that tt_local_entry is still hashed?
> 
> batadv_tt_local_remove() gets the entry from the lockless
> batadv_tt_local_hash_find(). Nothing checks the entry again once
> list_lock is held. Consider:
> 
> 1. Entry E is already PENDING with a DEL queued, for example from
>    batadv_tt_local_purge_list().
> 2. CPU0: batadv_tt_local_remove() finds E and takes a reference.
> 3. CPU1: a commit runs batadv_tt_local_purge_pending_clients(), which
>    calls hlist_del_rcu() on E. The first DEL is sent.
> 4. batadv_tt_local_add() creates E' for the same addr/vid and queues its
>    ADD.
> 5. CPU0 takes list_lock and sees that E isn't NEW. It sets PENDING again
>    and queues a second DEL for E.
> 
> batadv_tt_local_event() only matches on addr and vid:
> 
>         del_op_entry = entry->change.flags & BATADV_TT_CLIENT_DEL;
>         if (del_op_requested != del_op_entry) {
>             ...
>             list_del(&entry->list);
>             kmem_cache_free(batadv_tt_change_cache, entry);
>             changes--;
> 
> So the stale DEL cancels the ADD for E'. E' stays in the local hash and
> counts toward the CRC after the next batadv_tt_local_transition_new(),
> but it is never announced. Neighbours then see a CRC mismatch and need a
> full table request.
> 
> batadv_tt_local_remove_now() already guards against this. It queues its
> DEL only after batadv_hash_remove() confirms that this exact object was
> unlinked. That check came from the earlier series commit "batman-adv: tt:
> only queue local del event after successful unlink".
> 
> Would a hlist_unhashed() check under list_lock be the equivalent here?

The list_lock is already held and thus the unhashed check should be safe.


Related issues 1 (1 open — 0 closed)

Related to batman-adv - Bug #482: tt: don't queue local events for unhashed entriesIn Progressbatman-adv developers10/02/2026

Actions
Actions #1

Updated by Sven Eckelmann 15 days ago

  • Related to Bug #482: tt: don't queue local events for unhashed entries added
Actions #2

Updated by Sven Eckelmann 15 days ago

Actions

Also available in: Atom PDF