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

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

On 9/28/26 5:33 PM, sashiko-bot@kernel.org wrote:
> 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?

Okay, commit "bpf: Retarget indirect jump targets across prologue prepends"
introduced BPF_PATCH_MOVE_TARGET and merged last Friday. It solved three
cases for ops->gen_epilogue, ops->gen_prologue || env->seen_direct_write, and
stack slots for subprogs. ops->gen_epilogue has been rejected in patch 5.
We cannot allow ops->gen_epilogue since it may silently exit.
For other cases in "bpf: Retarget indirect jump targets across prologue prepends",
The above commit should already handle this.

For the other two, cleanup_pad == off + 1 cannot happen, because MOVE only
patches an entry insn and a landing pad cannot start at insn 0: the entry is
always walked outside a pad first, so reaching it again from an unwind
fails bpf_exc_check_insn() with "insn %u runs both inside and outside a
landing pad". Pads after off are shifted by the existing `> off + 1`, and a
covered call at off moves with its insn_aux_data.

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