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

[sashiko-bot@kernel.org]

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929001601.3242665-1-yonghong.song@linux.dev?part=5