Re: [PATCH bpf-next v11 04/26] bpf: Keep a call site's landing pad in insn_aux_data, add lookups

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

On 10/14/26 9:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: adjust_insn_aux_data misses cleanup_pad on BPF_PATCH_MOVE_TARGET
> --
>
> commit 407d61973b5e3b7df47dda20c8cb86db0ad2e4ca
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Keep a call site's landing pad in insn_aux_data, add lookups
>
> A call site's landing pad, if any, is kept in insn_aux_data as
> cleanup_pad. adjust_insn_aux_data(), which every instruction patch goes
> through, and verifier_remove_insns() keep it in step, and
> bpf_opt_remove_nops() moves a pad off a nop to the next instruction
> before removing the nop. Add lookups for it and for calls to
> bpf_unwind() and bpf_unwind_resume(); their users come in later patches.
>
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index b2fa7e8bedf3a..18c563abd988f 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
> [ ... ]
>> @@ -244,11 +244,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: High]
> If bpf_convert_ctx_accesses() inserts a prologue at the start of a subprogram
> (offset 0) by calling __bpf_patch_insn_data() with BPF_PATCH_MOVE_TARGET,
> the original instruction shifts from index 0 down to index cnt - 1.
>
> Because of the strictly greater-than condition here (> off + 1), if a call
> site's landing pad is exactly at offset 0 (cleanup_pad == 1), the condition
> evaluates to false and the index won't be shifted.
>
> Could this cause the cleanup pad to point to the newly inserted prologue
> instead of the intended instruction, potentially leading to stack or
> context corruption during an exception unwind?

As mentioned in previous comments, landing_pad cannot be in the beginning
of a subprogram. For example:
    landing_pad_start:
       insn 0: r1 = r6
               ...
               call bpf_unwind_resume

The verifier will go to insn 0 and assuming it is NOT in landing_pad
and eventually it will hit bpf_unwind_resume() and the verifier will
reject it since it thinks bpf_unwind_resume() is out of landing pad.

This patch should be okay.

>