Re: [PATCH bpf-next v6 05/21] bpf: Prepare for an exception cleanup table before the CFG walk

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

On 9/25/26 10:16 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] bpf: bpf_throw() bypasses exception_callback_subprog check skipping cleanup pads
> --
>
> commit bfb14bc84bb67f59ae10ca034e824ec0357d7d80
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Prepare for an exception cleanup table before the CFG walk
>
> This commit introduces bpf_prepare_cleanup_exceptions() to record landing pads
> for BPF-to-BPF calls and bpf_unwind() within a cleanup record's range. It
> ensures the JIT supports dispatching landing pads and refuses to combine
> tables with an exception callback.
>
>> diff --git a/kernel/bpf/exception.c b/kernel/bpf/exception.c
>> index b19fcbf49b7ef..1ed0370a171b7 100644
>> --- a/kernel/bpf/exception.c
>> +++ b/kernel/bpf/exception.c
>> @@ -7,9 +7,56 @@
>>   #include <linux/filter.h>
>>   #include "exception.h"
>>   
>> +#define verbose(env, fmt, args...) bpf_verifier_log_write(env, fmt, ##args)
>> +
>>   BTF_ID_LIST_SINGLE(bpf_unwind_id, func, bpf_unwind)
>>   BTF_ID_LIST_SINGLE(bpf_unwind_resume_id, func, bpf_unwind_resume)
>>   
>> +static void mark_call_sites(struct bpf_verifier_env *env)
>> +{
>> +	u32 i, j;
>> +
>> +	for (i = 0; i < env->cleanup_info_cnt; i++) {
>> +		struct bpf_cleanup_info *rec = &env->cleanup_info[i];
>> +
>> +		for (j = rec->begin_off; j < rec->end_off; j++) {
>> +			struct bpf_insn *insn = &env->prog->insnsi[j];
>> +
>> +			if (!bpf_pseudo_call(insn) && !bpf_is_unwind_kfunc(insn))
>> +				continue;
> [Severity: Medium]
> Since bpf_throw() is not matched here, does it skip receiving a landing pad?
>
> If a program is loaded with .bpf_cleanup records and calls bpf_throw() without
> specifying a custom exception callback, could this cause it to silently bypass
> the cleanup execution during unwinding, leaking user-defined logical resources
> like map-based counters?

Yes, when bpf_throw() is triggered and landing pad has some resource to be
released, it is possible those resource is not released. Will reject
bpf_throw() if *any* landing pad exists in the program.

>
>> +			env->insn_aux_data[j].cleanup_pad = rec->landing_pad_off + 1;
>> +		}
>> +	}
>> +}
>> +
>> +int bpf_prepare_cleanup_exceptions(struct bpf_verifier_env *env)
>> +{
> [ ... ]
>
>> +	if (!bpf_jit_supports_cleanup_pads() || !env->prog->jit_requested) {
>> +		verbose(env,
>> +			"exception cleanup needs a JIT that can dispatch landing pads\n");
>> +		return -EOPNOTSUPP;
>> +	}
>> +	env->prog->jit_required = 1;
>> +
>> +	if (env->exception_callback_subprog) {
> [Severity: Medium]
> Does this check the state of env->exception_callback_subprog too early?
>
> If a later pass dynamically inserts a default exception callback, could it
> bypass this restriction and result in a mix of exception cleanup tables and
> bpf_throw() usage?

In the beginning of bpf_prepare_cleanup_exceptions(), we have

+       if (!env->cleanup_info_cnt)
+               return 0;

so landing_pad will exist in the prog. With previous explanation, that
means bpf_throw() will be rejected. So reject exception_callback_subprog
too.

>
>> +		verbose(env,
>> +			"exception cleanup table cannot be combined with an exception callback\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	mark_call_sites(env);
>> +	return 0;
>> +}