Re: [PATCH net-next 08/15] batman-adv: tt: replace add_orig_entry goto in batadv_tt_global_add()

[Sven Eckelmann <sven@narfation.org>]

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

On Tuesday, 8 September 2026 10:27:01 CEST sashiko-bot@kernel.org wrote:
[...]
> This commit refactors batadv_tt_global_add() by removing the add_orig_entry
> label and turning the flag merging and list purging logic into a more
> direct if/else structure.
> 
> > diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
> > index 67e4ade5ae997..760acc7be973e 100644
> > --- a/net/batman-adv/translation-table.c
> > +++ b/net/batman-adv/translation-table.c
> > @@ -2021,15 +2021,16 @@ static bool batadv_tt_global_add(struct batadv_priv *bat_priv,
> >  			if (batadv_tt_global_entry_has_orig(tt_global_entry,
> >  							    orig_node, NULL))
> >  				goto out_remove;
> > -			batadv_tt_global_del_orig_list(tt_global_entry);
> > -			goto add_orig_entry;
> > +
> > +			delete = true;
> > +		} else {
> > +			delete = batadv_tt_global_merge_flags(tt_global_entry, flags);
> >  		}
> >  
> > -		delete = batadv_tt_global_merge_flags(tt_global_entry, flags);
> >  		if (delete)
> >  			batadv_tt_global_del_orig_list(tt_global_entry);
> 
> [Severity: High]
> This is a pre-existing issue, but this patch preserves and refactors the
> vulnerable code structure without resolving an underlying race condition.
> 
> Could this lead to a Time-of-Check to Time-of-Use race that permanently
> drops a valid originator from the global translation table? 
> 
> For example, if two CPUs concurrently execute batadv_tt_global_add() for the
> same translation table entry:
> 
> CPU 0 (adding a temporary client):
> Evaluates global_flags without locking and sets delete = true.
> 
> CPU 1 (adding a non-temporary client):
> Merges its non-temporary flags, clears the TEMP flag, purges the list, and
> adds its non-temporary originator.
> 
> CPU 0 (resuming based on its stale delete snapshot):
> Incorrectly purges CPU 1's newly added non-temporary originator and adds
> its own temporary one.
> 
> Because the TEMP flag was cleared by CPU 1, the entry might never timeout or
> update correctly, potentially causing a permanent routing blackhole for the
> affected client.
> 
> A similar race appears to exist between two non-temporary additions. If CPU 1
> is preempted after batadv_tt_global_merge_flags() returns true, CPU 2 might
> return false from the merge and add its originator. When CPU 1 resumes, it
> would purge CPU 2's originator based on the stale delete boolean.
> 
> Is additional synchronization needed around the delete decision and the list
> purging to prevent these concurrent additions from stepping on each other?

Same as last time: https://www.open-mesh.org/issues/474

Regards,
	Sven

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