Re: [PATCH bpf-next v2 1/2] bpf: Keep target extended until its last freplace link detaches

[Jiri Olsa <olsajiri@gmail.com>]

On Thu, Sep 24, 2026 at 03:06:16PM +0800, Leon Hwang wrote:
> On 24/9/26 10:37, chenyuan_fl@163.com wrote:
> > From: Yuan Chen <chenyuan@kylinos.cn>
> > 
> > The is_extended / prog_array_member_cnt protocol introduced by commit
> > d6083f040d5d ("bpf: Prevent tailcall infinite loop caused by freplace")
> > keeps a prog extended by a freplace program out of prog_array maps, and
> > vice versa: once a tail call re-enters an extended subprogram, its
> > tail_call_cnt resets on every execution and the loop never terminates.
> > 
> > But is_extended is a plain boolean, while one target prog can carry
> > several freplace links at the same time, one on its entry and one on a
> > global subprogram.  __bpf_trampoline_unlink_prog() cleared is_extended
> > whenever *any* freplace link detached, so detaching one of two links
> > re-armed the unbounded loop through the remaining one.
> 
> A prog may have multiple global subprogs. And each of the subprogs can
> be attached with freplace prog. When all the subprogs are attached with
> freplace progs then detach one of the freplace prog, the *is_extended*
> becomes *false*, which relaxes the restriction between tailcall and
> freplace introduced by the commit d6083f040d5d ("bpf: Prevent tailcall
> infinite loop caused by freplace").
> 
> > 
> > Replace the is_extended boolean with a count of the freplace links
> > attached to each target prog, so the target stays extended until its
> > last link detaches.  Also rename bpf_freplace_check_tgt_prog() to
> > bpf_freplace_link_tgt_prog(), as the helper has never been a pure
> > check: it reserves the target prog on success.
> > 
> > Fixes: d6083f040d5d ("bpf: Prevent tailcall infinite loop caused by freplace")
> > Signed-off-by: Yuan Chen <chenyuan@kylinos.cn>
> > 
> 
> [...]
> 
> > @@ -933,7 +933,7 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_node *node,
> >  		/* Cannot attach extension if fentry/fexit are in use. */
> >  		if (cnt)
> >  			return -EBUSY;
> > -		err = bpf_freplace_check_tgt_prog(tgt_prog);
> > +		err = bpf_freplace_link_tgt_prog(tgt_prog);
> >  		if (err)
> >  			return err;
> >  		tr->extension_prog = node->link->prog;
> 
>                 return bpf_arch_text_poke(tr->func.addr, BPF_MOD_NOP,
>                                           BPF_MOD_JUMP, NULL,
>                                           node->link->prog->bpf_func);
> 
> 
> I think there are existing issues here: if bpf_arch_text_poke() returns
> error, the tr->extension_prog and aux->freplace_link_cnt should be
> rollbacked.

+1, let's set tr->extension_prog and tgt_prog->aux->freplace_link_cnt++
only if bpf_arch_text_poke succeeds

jirka

> 
> Thanks,
> Leon
> 
> > @@ -979,7 +979,7 @@ static int __bpf_trampoline_unlink_prog(struct bpf_tramp_node *node,
> >  					 tr->extension_prog->bpf_func, NULL);
> >  		tr->extension_prog = NULL;
> >  		guard(mutex)(&tgt_prog->aux->ext_mutex);
> > -		tgt_prog->aux->is_extended = false;
> > +		tgt_prog->aux->freplace_link_cnt--;
> >  		return err;
> >  	}
> >  	bpf_trampoline_remove_prog(tr, node);
> 
>