Re: [PATCH net-next 09/15] batman-adv: tt: extract code handling a roam on add

[Sven Eckelmann <sven@narfation.org>]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Tuesday, 1 September 2026 16:00:01 CEST sashiko-bot@kernel.org wrote:
[...]
> [Severity: High]
> This is a pre-existing issue, but I'm wondering if the handling of pending
> and roamed clients in batadv_tt_local_add() might have a logical flaw that
> persists here.
> 
> When batadv_tt_local_remove() is called with roaming=true for a fully
> synced client, it sets both BATADV_TT_CLIENT_ROAM and
> BATADV_TT_CLIENT_PENDING.
> 
> If the client roams back before the entry is purged, calling
> batadv_tt_local_add(), it hits this earlier code path:
> 
>     if (tt_local->common.flags & BATADV_TT_CLIENT_PENDING) {
>         ...
>         tt_local->common.flags &= ~BATADV_TT_CLIENT_PENDING;
>         spin_unlock_bh(&tt_local->common.flags_lock);
> 
>         goto add_event;
>     }
> 
>     if (tt_local->common.flags & BATADV_TT_CLIENT_ROAM) {
>         ...
>         tt_local->common.flags &= ~BATADV_TT_CLIENT_ROAM;
>         roamed_back = true;
>     }
> 
> Because of the goto add_event, it bypasses the BATADV_TT_CLIENT_ROAM
> check entirely, skipping roamed_back = true. It then proceeds from
> add_event into check_roaming, where batadv_tt_local_add_roam() is
> called with roamed_back == false.
> 
> Can this result in the local TT entry remaining incorrectly marked with
> BATADV_TT_CLIENT_ROAM, causing packets destined for the returned client to
> be misrouted or dropped?
> 
> Does this also cause the global roaming state to be incorrectly preserved
> instead of being properly canceled?
> 
> 

Out of scope for this patch. Documented it for interested contributors in 
https://www.open-mesh.org/issues/472

Regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 228 bytes --]