Re: [PATCH bpf-next v10 11/24] bpf: Prepare JITed programs for dispatching cleanup pads

[Yonghong Song <yonghong.song@linux.dev>]

On 10/9/26 6:35 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: backward jump offsets in bpf_exc_patch_unwind_calls are corrupted by bpf_adj_branches
> --
>
> commit b2a75ebf7c80cc3231a5eb34881ba1928dafcd3b
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Prepare JITed programs for dispatching cleanup pads
>
> This patch prepares JITed programs to dispatch exception cleanup pads.
> It updates the verifier to patch bpf_unwind() calls with a sequence that
> clears r0 and jumps to the exception pad or exits.
>
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index 64baee1a37e60..1cc025c45ecda 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
> [ ... ]
>> @@ -1733,6 +1805,48 @@ static int may_goto_expand(struct bpf_insn *insn_buf, int off, int stack_off,
> [ ... ]
>> +/*
>> + * Follow each bpf_unwind() call with 'r0 = 0; exit', or with
>> + * 'r0 = 0; goto pad' where a record covers the call.
>> + */
>> +int bpf_exc_patch_unwind_calls(struct bpf_verifier_env *env)
>> +{
>> +	int insn_cnt = env->prog->len;
>> +	struct bpf_insn insn_buf[3];
>> +	struct bpf_prog *new_prog;
>> +	int i, off, delta = 0;
>> +
>> +	if (!bpf_prog_may_unwind(env))
>> +		return 0;
>> +
>> +	for (i = 0; i < insn_cnt; i++) {
>> +		int call = i + delta, pad;
>> +		struct bpf_insn *insn = env->prog->insnsi + call;
>> +
>> +		if (!bpf_is_unwind_kfunc(insn))
>> +			continue;
>> +
>> +		insn_buf[0] = *insn;
>> +		insn_buf[1] = BPF_MOV64_IMM(BPF_REG_0, 0);
>> +		insn_buf[2] = BPF_EXIT_INSN();
>> +		pad = bpf_exc_pad_of_call(env, call);
>> +		if (pad >= 0) {
>> +			/* The goto is at call + 2; a pad after the call moves down by 2. */
>> +			if (pad > call)
>> +				pad += 2;
>> +			off = pad - (call + 3);
>> +			insn_buf[2] = off == (s16)off ? BPF_JMP_A(off) : BPF_JMP32_A(off);
> [Severity: High]
> Does this manually calculated offset get corrupted if the landing pad is located
> before the unwind call (pad < call)?

I think this cannot happen. See below.

>
> bpf_patch_insn_data() is called immediately after this, which delegates to
> bpf_adj_branches() to adjust all relative offsets in the program.
>
> When bpf_adj_branches() processes the newly inserted goto instruction, it
> evaluates it as if it were a shifted original instruction (because
> curr >= end_new evaluates to true). If the jump is backwards,
> bpf_adj_branches() might erroneously subtract the insertion delta (2) from the
> already-correct offset.
>
> Could this result in the jump target being shifted 2 instructions before the
> actual landing pad, potentially leading to the execution of unintended or
> unverified bytecode during exception unwinding?

The offset is already right, and no change for bpf_adj_branches().
The goto is inside the patch [call, call + 3). bpf_adj_delta_to_off()
only rewrites a jump that sits before pos or at or after end_new,
i.e. one that was outside the patch.

>
>> +		}
>> +
>> +		new_prog = bpf_patch_insn_data(env, call, insn_buf, 3);
>> +		if (!new_prog)
>> +			return -ENOMEM;
>> +		delta += 2;
>> +		env->prog = new_prog;
>> +	}
>> +	return 0;
>> +}