Re: [PATCH bpf-next v3 03/20] bpf: Add lookups for exception cleanup resumes and landing pads

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

On 9/19/26 11:31 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index c08505b9ba82..f9bccd3e0f4d 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -681,6 +681,11 @@ struct bpf_insn_aux_data {
>>   	bool needs_zext; /* alu op needs to clear upper bits */
>>   	bool non_sleepable; /* helper/kfunc may be called from non-sleepable context */
>>   	bool is_iter_next; /* bpf_iter_<type>_next() kfunc call */
>> +	/*
>> +	 * 1 + the instruction index of the exception cleanup landing pad this
>> +	 * call site unwinds to, or 0 for none.
>> +	 */
>> +	u32 cleanup_pad;
>>   	bool call_with_percpu_alloc_ptr; /* {this,per}_cpu_ptr() with prog percpu alloc */
>>   	u8 alu_state; /* used in combination with alu_limit */
>>   	/* true if STX or LDX instruction is a part of a spill/fill
> [ ... ]
>
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index 2add8001c3ec..dd1a6b5a6afd 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
>> @@ -261,6 +261,11 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env,
>>   		}
>>   	}
>>
>> +	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;
>> +
> Does this miss the case where the original instruction is not placed last
> in the patch buffer? Looking at adjust_insn_aux_data(), there's logic
> earlier in the function that handles non_stack_access when the patched
> instructions contain the original:
>
> kernel/bpf/fixups.c:adjust_insn_aux_data() {
>      for (i = off; i < off + cnt - 1; i++) {
>          /* Expand insni[off]'s seen count to the patched range. */
>          data[i].seen = old_seen;
>          data[i].zext_dst = bpf_insn_def32(new_prog, insn + i) >= 0;
>          if (!memcmp(insn + i, original_insn, sizeof(struct bpf_insn))) {
>              data[i].non_stack_access =
>                  data[off + cnt - 1].non_stack_access;
>              data[off + cnt - 1].non_stack_access = false;
>          } else if (bpf_is_mem_insn(insn + i)) {
>              data[i].non_stack_access = true;
>          }
>      }
> }
>
> The memcmp branch transfers non_stack_access from data[off + cnt - 1] to
> data[i] when it finds the original instruction. A bit further down,
> indirect_target gets similar treatment.
>
> cleanup_pad is a property of one specific call instruction (the landing
> pad this call site unwinds to), not of an instruction slot. When
> bpf_patch_insn_data() parks the original instruction's aux record in
> data[off + cnt - 1] but the patch buffer places the original instruction
> at position i (not last), shouldn't cleanup_pad be transferred the same
> way?
>
> Without this transfer, after such a patch the surviving call instruction
> would have cleanup_pad == 0 (no landing pad) and an unrelated newly
> generated instruction would carry the pad index instead. The CFG edge from
> the call to its pad would disappear, and an exception unwinding through
> that frame would silently skip its cleanup.
>
> A later commit (d710bf82c5e9, "bpf: Dispatch exception cleanup pads from
> bpf_throw()") adds these lines to the memcmp branch:
>
>      data[i].cleanup_pad = data[off + cnt - 1].cleanup_pad;
>      data[off + cnt - 1].cleanup_pad = 0;
>
> Should that transfer belong in this commit, which introduces the field and
> claims to keep it in step?

You are right. We can transfer the above cleanup_pad in this commit to keep
the field consistent.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35492765538