Re: [PATCH bpf-next v7 04/22] bpf: Add lookups for exception cleanup resumes and landing pads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: missing cleanup_pad adjustment in BPF_PATCH_MOVE_TARGET
--

commit ddeb49ab1c3e86d5fea9b3b127432b6e34db617b
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Add lookups for exception cleanup resumes and landing pads

This commit introduces helpers to recognize exception cleanup resumes and
landing pads. It also adds logic to maintain cleanup_pad data in
insn_aux_data when instructions are patched or removed.

> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
> index 37cf130ebb57b..5b7fe4ba610b3 100644
> --- a/kernel/bpf/fixups.c
> +++ b/kernel/bpf/fixups.c
> @@ -268,11 +268,18 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
>  			data[i].non_stack_access =
>  				data[off + cnt - 1].non_stack_access;
>  			data[off + cnt - 1].non_stack_access = false;
> +			data[i].cleanup_pad = data[off + cnt - 1].cleanup_pad;
> +			data[off + cnt - 1].cleanup_pad = 0;
>  		} else if (bpf_is_mem_insn(insn + i)) {
>  			data[i].non_stack_access = true;
>  		}
>  	}
>  
> +	if (env->cleanup_info_cnt)
> +		for (i = 0; i < prog_len; i++)
> +			if (data[i].cleanup_pad > off + 1)
> +				data[i].cleanup_pad += cnt - 1;

[Severity: Medium]
When mode is BPF_PATCH_MOVE_TARGET and the landing pad is located exactly
at the patched instruction, does this condition fail to adjust it?

Since cleanup_pad is 1-based, a value of off + 1 means the landing pad is
exactly at off. Because this strictly checks > off + 1, it skips adjusting
data[i].cleanup_pad when it equals off + 1. 

If BPF_PATCH_MOVE_TARGET is used (for example, with gen_prologue or may_goto
initialization), the original instruction is moved to the end of the patch
sequence. This means the landing pad should follow it and move to the end
as well, but it will be left pointing to the start of the inserted sequence.

Could this result in the unwinder jumping to the newly inserted prologue or
loop initialization instead of the actual exception landing pad?

> +
>  	/*
>  	 * Last slot instruction could be a newly generated
>  	 * BPF_ST/BPF_LDX/BPF_STX, systematically mark it for non-stack access

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