Re: [PATCH bpf-next v7 05/22] bpf: Prepare for an exception cleanup table before the CFG walk

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

On 9/28/26 5:31 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: cleanup_pad adjustment fails for BPF_PATCH_MOVE_TARGET at off=0
> --
>
> commit fc0420a45de5f3cee47a65d96e78244ace4fbf4e
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Prepare for an exception cleanup table before the CFG walk
>
> This patch prepares an exception cleanup table for BPF programs, adding
> landing pads to the instruction auxiliary data for calls that can unwind.
> It introduces the cleanup_pad field to track where execution should resume
> during exception unwinding.
>
>> diff --git a/kernel/bpf/exception.c b/kernel/bpf/exception.c
>> index b19fcbf49b7ef..c1779d2d02f05 100644
>> --- a/kernel/bpf/exception.c
>> +++ b/kernel/bpf/exception.c
> [ ... ]
>
>> +static int mark_call_sites(struct bpf_verifier_env *env)
>> +{
>> +	u32 i, j;
>> +
>> +	for (i = 0; i < env->cleanup_info_cnt; i++) {
>> +		struct bpf_cleanup_info *rec = &env->cleanup_info[i];
>> +
>> +		for (j = rec->begin_off; j < rec->end_off; j++) {
>> +			struct bpf_insn *insn = &env->prog->insnsi[j];
>> +
>> +			if (!bpf_pseudo_call(insn) && !bpf_is_callx(insn) &&
>> +			    !bpf_is_unwind_kfunc(insn))
>> +				continue;
>> +			env->insn_aux_data[j].cleanup_pad = rec->landing_pad_off + 1;
> [Severity: High]
> How does this new cleanup_pad field handle instruction patching when a
> landing pad is located at instruction 0?
>
> If mark_call_sites() sets cleanup_pad = rec->landing_pad_off + 1, and the
> landing pad is at index 0, cleanup_pad becomes 1.
>
> If bpf_convert_ctx_accesses() in kernel/bpf/fixups.c later inserts a
> prologue at instruction 0 using BPF_PATCH_MOVE_TARGET,
> adjust_insn_aux_data() will try to update the aux data offsets. However,
> its check:
>
>      if (data[i].cleanup_pad > off + 1)
>          data[i].cleanup_pad += cnt - 1;
>
> will evaluate to 1 > 1 (false) when off=0. The required shift is skipped,
> leaving the cleanup_pad pointing to the newly inserted prologue instead of
> shifting to the original instruction. Unwinding to this pad could re-execute
> the prologue and potentially corrupt the frame pointer or stack.
>
> Can this sequence occur and lead to stack issues?

The landing pad cannot be at insn 0. The prog entry starts with non-pad and
until it finds bpf_unwind() and goes to landing pad. In such cases, you will
see landing_pad insn with non-landing-pad and with-landing-pad. This will
be rejected.

>
>> +		}
>> +	}
>> +	return 0;
>> +}