Re: [PATCH bpf-next v3 01/10] bpf: propagate original instruction offset when patching program

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] State leakage of `insn_off_in_patch` across loop iterations in `bpf_convert_ctx_accesses`
--

commit 37bd4077be7be826caa9dd6664d363fc86f4d0e5
Author: Alexis Lothoré (eBPF Foundation) <alexis.lothore@bootlin.com>

bpf: propagate original instruction offset when patching program

This commit adds a new parameter to bpf_patch_insn_data and
adjust_insn_aux_data to convey the location of the original instruction
within the newly generated patch. This metadata maps the original
instruction to the patch for future KASAN instrumentation.

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 3cf2cc6e3ab66..1f340211b65c2 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -695,6 +697,7 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
>  	struct bpf_insn *insn_buf = env->insn_buf;
>  	struct bpf_insn *insn;
>  	u32 target_size, size_default, off;
> +	s32 insn_off_in_patch = -1;
>  	struct bpf_prog *new_prog;
>  	enum bpf_access_type type;
>  	bool is_narrower_load;

[ ... ]

> @@ -856,16 +862,20 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
>  			if (!ops->convert_ctx_access)
>  				continue;
>  			convert_ctx_access = ops->convert_ctx_access;
> +			insn_off_in_patch = 0;
>  			break;

[ ... ]

> @@ -971,7 +981,8 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
>  						       size * 8, 0);
>  
>  patch_insn_buf:
> -		new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
> +		new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt,
> +					       insn_off_in_patch);

[Severity: Medium]
Does this leak the insn_off_in_patch state across loop iterations?

Because insn_off_in_patch is initialized outside the main instruction
processing loop, if a context access like PTR_TO_CTX sets it to 0, it
is not reset back to -1 for subsequent iterations.

If an epilogue-generating instruction like BPF_EXIT follows later and takes
the goto patch_insn_buf path:

bpf_convert_ctx_accesses() {
    ...
    } else if (insn->code == (BPF_JMP | BPF_EXIT) &&
               epilogue_cnt &&
               i + delta < subprogs[1].start) {
        ...
        goto patch_insn_buf;

Won't this erroneously reuse the stale 0 value instead of -1? This could
cause adjust_insn_aux_data() to incorrectly map metadata for the epilogue.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701-kasan-v3-0-bd09bb942d86@bootlin.com?part=1