Re: [PATCH bpf-next v2 10/20] bpf: Dispatch exception cleanup pads from bpf_throw()

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

On 9/17/26 10:58 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/exception.c b/kernel/bpf/exception.c
>> index b2bf831242b85..521086d084a35 100644
>> --- a/kernel/bpf/exception.c
>> +++ b/kernel/bpf/exception.c
> [ ... ]
>
>> +static int cmp_u32(const void *a, const void *b)
>> +{
>> +	u32 x = *(const u32 *)a, y = *(const u32 *)b;
>> +
>> +	return x < y ? -1 : x > y;
>> +}
> [ ... ]
>
>> +/* Is @idx in the sorted array @at of @n instruction indices? */
>> +static bool insn_idx_in(const u32 *at, u32 n, u32 idx)
>> +{
>> +	u32 l = 0, r = n;
>> +
>> +	while (l < r) {
>> +		u32 m = l + (r - l) / 2;
>> +
>> +		if (idx < at[m])
>> +			r = m;
>> +		else if (idx > at[m])
>> +			l = m + 1;
>> +		else
>> +			return true;
>> +	}
>> +	return false;
>> +}
> This isn't a bug, but could insn_idx_in() just use bsearch() from
> <linux/bsearch.h>, reusing the cmp_u32() already defined above for the
> sort()? kernel/bpf/fixups.c in this series uses bsearch() for similar
> lookups.

Good point. will do.

>
>> diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
>> index f120ae66dbbcb..134aafa6a6c9b 100644
>> --- a/kernel/bpf/fixups.c
>> +++ b/kernel/bpf/fixups.c
> [ ... ]
>
>> @@ -1113,6 +1120,116 @@ static void bpf_restore_subprog_starts(struct bpf_verifier_env *env, u32 *orig_s
>>   	env->subprog_info[env->subprog_cnt].start = env->prog->len;
>>   }
>>
>> +static int cleanup_info_for_subprog(struct bpf_verifier_env *env, struct bpf_prog *sub,
>> +				    u32 start, u32 end)
>> +{
>> +	struct bpf_cleanup_info *recs;
>> +	u32 i, cnt = 0;
>> +	int err;
>> +
>> +	if (!env->cleanup_info_cnt)
>> +		return 0;
>> +
>> +	err = bpf_cleanup_alloc_info(sub->aux);
>> +	if (err)
>> +		return err;
>> +
>> +	err = cleanup_throw_sites_for_subprog(env, sub, start, end);
>> +	if (err)
>> +		return err;
>> +
>> +	err = cleanup_pad_body_for_subprog(env, sub, start, end);
>> +	if (err)
>> +		return err;
>> +
>> +	for (i = start; i < end; i++)
>> +		if (env->insn_aux_data[i].cleanup_pad)
>> +			cnt++;
>> +	if (!cnt)
>> +		return 0;
>> +
>> +	recs = kvmalloc_array(cnt, sizeof(*recs), GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
>> +	if (!recs)
>> +		return -ENOMEM;
>> +
>> +	for (i = start, cnt = 0; i < end; i++) {
>> +		u32 pad = env->insn_aux_data[i].cleanup_pad;
>> +
>> +		if (!pad)
>> +			continue;
>> +		pad--;
>> +		if (verifier_bug_if(pad < start || pad >= end, env,
>> +				    "insn %u is covered by a landing pad at %u outside its subprog [%u, %u)",
>> +				    i, pad, start, end)) {
>> +			kvfree(recs);
>> +			return -EFAULT;
>> +		}
>> +		recs[cnt].begin_off = i - start;
>> +		recs[cnt].end_off = i - start + 1;
>> +		recs[cnt].landing_pad_off = pad - start;
>> +		cnt++;
>> +	}
>> +	return bpf_cleanup_attach_info(sub->aux, recs, cnt);
>> +}
> [ ... ]
>
>>   static int jit_subprogs(struct bpf_verifier_env *env)
>>   {
>>   	struct bpf_prog *prog = env->prog, **func, *tmp;
> [ ... ]
>
>> @@ -1250,6 +1367,10 @@ static int jit_subprogs(struct bpf_verifier_env *env)
>>   		func[i]->aux->token = prog->aux->token;
>>   		if (!i)
>>   			func[i]->aux->exception_boundary = env->seen_exception;
>> +		err = cleanup_info_for_subprog(env, func[i], subprog_start,
>> +					       env->subprog_info[i + 1].start);
> This isn't a bug, but would passing subprog_end (already computed a few
> lines above from env->subprog_info[i + 1].start) be clearer here? It
> keeps the cleanup table's end bound explicitly tied to the same value
> jit_subprogs() uses for func[i]->len.

Good point. subprog_end is a good choice.

>
>
> ---
> 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/35308528711