Re: [PATCH bpf-next] bpf: Roll back freplace link state when bpf_arch_text_poke() fails

[Leon Hwang]

On 25/9/26 12:35, chenyuan_fl@xxxxxxx wrote:
> From: Yuan Chen <chenyuan@xxxxxxxxxx>
> 
> A freplace attach claims the target prog by bumping
> tgt_prog->aux->freplace_link_cnt and setting tr->extension_prog.
> bpf_arch_text_poke() then makes the extension take effect.  If the
> poke fails, the claims are never released: the attach unwinds
> through bpf_link_cleanup(), which clears link->prog, so
> bpf_trampoline_unlink_prog() never runs.
> 
> Drop the link count under ext_mutex on the error path, and set
> tr->extension_prog only after the poke succeeded.  The count is
> still bumped before the poke: it blocks prog_array updates while
> the entry is patched.
> 
> Fixes: d6083f040d5d ("bpf: Prevent tailcall infinite loop caused by freplace")

Your commit msg also says the issue about tr->extension_prog.

An extra Fixes tag for tr->extension_prog should be added.

Fixes: be8704ff07d2 ("bpf: Introduce dynamic program extensions")

The changes below lgtm:

Acked-by: Leon Hwang <leon.hwang@xxxxxxxxx>

Thanks,
Leon

> Suggested-by: Leon Hwang <leon.hwang@xxxxxxxxx>
> Signed-off-by: Yuan Chen <chenyuan@xxxxxxxxxx>
> ---
>  kernel/bpf/trampoline.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/trampoline.c b/kernel/bpf/trampoline.c
> index da85bd580ef0..bf4ab0ac264e 100644
> --- a/kernel/bpf/trampoline.c
> +++ b/kernel/bpf/trampoline.c
> @@ -973,10 +973,17 @@ static int __bpf_trampoline_link_prog(struct bpf_tramp_node *node,
>  		err = bpf_freplace_link_tgt_prog(tgt_prog);
>  		if (err)
>  			return err;
> +		err = bpf_arch_text_poke(tr->func.addr, BPF_MOD_NOP,
> +					 BPF_MOD_JUMP, NULL,
> +					 node->link->prog->bpf_func);
> +		if (err) {
> +			/* Undo the claim from bpf_freplace_link_tgt_prog(). */
> +			guard(mutex)(&tgt_prog->aux->ext_mutex);
> +			tgt_prog->aux->freplace_link_cnt--;
> +			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);
> +		return 0;
>  	}
>  	err = bpf_trampoline_add_prog(tr, node, cnt);
>  	if (err)