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

[sashiko-bot@kernel.org]

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?

> +			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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926050006.2213110-1-yonghong.song@linux.dev?part=5