Re: [PATCH v5 bpf-next 2/9] bpf: Adjust BPF_JMP that jumps to the 1st insn of the prologue

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2024-08-29 at 14:08 -0700, Martin KaFai Lau wrote:

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 261849384ea8..03e974129c05 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19286,6 +19286,9 @@ static int adjust_jmp_off(struct bpf_prog *prog, u32 tgt_idx, u32 delta)
>  	for (i = 0; i < insn_cnt; i++, insn++) {
>  		u8 code = insn->code;
>  
> +		if (tgt_idx <= i && i < tgt_idx + delta)
> +			continue;
> +
>  		if ((BPF_CLASS(code) != BPF_JMP && BPF_CLASS(code) != BPF_JMP32) ||
>  		    BPF_OP(code) == BPF_CALL || BPF_OP(code) == BPF_EXIT)
>  			continue;
> @@ -19704,6 +19707,9 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
>  		}
>  	}
>  
> +	if (delta)
> +		WARN_ON(adjust_jmp_off(env->prog, 0, delta));

Just noticed this.
Suppose prologue is three instructions long and no epilogue,
then cnt == 3 and delta == 2, adjust_jmp_off() would skip instructions
in range [0..2), while inserted instructions range is [0..2].
So, this would work only if the last statement in the prologue/epilogue
generator is:

	*insn++ = prog->insnsi[0];

which seems to be true for prologue generators in the tree,
but looks a bit unintuitive...

> +
>  	if (bpf_prog_is_offloaded(env->prog->aux))
>  		return 0;
>