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

[chenyuan <chenyuan_fl@163.com>]

Thanks for the review. This is a pre-existing bug in the link path, not introduced by this patch. I'll send a
separate fix for the rollback later. This patch stays focused on the is_extended counting.













At 2026-09-24 15:06:16, "Leon Hwang" <leon.hwang@linux.dev> 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.
>
>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);