Re: [PATCH bpf-next v3 01/20] bpf: Accept the compiler's exception cleanup table at program load

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: unbounded memory allocation in check_cleanup_info via cleanup_info_cnt
--

commit 18ab7230f0d26780fd3b40453aca2155a679f967
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Accept the compiler's exception cleanup table at program load

This commit adds support for BPF exception handling cleanup tables generated
by LLVM. It introduces a new UAPI to pass the .bpf_cleanup table into the
kernel via BPF_PROG_LOAD, along with the necessary validation logic in
check_cleanup_info().

Link: https://github.com/llvm/llvm-project/pull/192164 [1]

> diff --git a/kernel/bpf/check_btf.c b/kernel/bpf/check_btf.c
> index 0e8b3ccc7a5b9..d03dc791042a2 100644
> --- a/kernel/bpf/check_btf.c
> +++ b/kernel/bpf/check_btf.c

[ ... ]

> @@ -407,6 +407,149 @@ static int check_core_relo(struct bpf_verifier_env *env,
>  	return err;
>  }
>  
> +static int cleanup_insn_subprog(struct bpf_verifier_env *env, u32 off)
> +{

[ ... ]

> +static int check_cleanup_info(struct bpf_verifier_env *env,
> +			      const union bpf_attr *attr,
> +			      bpfptr_t uattr)
> +{
> +	u32 krec_size = sizeof(struct bpf_cleanup_info);
> +	u32 i, nrec, urec_size, min_size, prev_end = 0;
> +	struct bpf_cleanup_info *krecord;
> +	bpfptr_t urecord;
> +	int ret = -EINVAL;
> +
> +	nrec = attr->cleanup_info_cnt;
> +	if (!nrec)
> +		return 0;
> +	if (nrec > INT_MAX / krec_size)
> +		return -EINVAL;

[Severity: Medium]
Can an unprivileged user trigger a multi-gigabyte memory allocation here
by passing an artificially large attr->cleanup_info_cnt?

While the allocation below uses GFP_KERNEL_ACCOUNT and will not cause a CPU
lockup, kvcalloc() still attempts to allocate and zero up to ~2.1 GB of
memory.

Since each cleanup info record must cover at least one instruction and
they cannot overlap, is there a case where nrec could legitimately
exceed env->prog->len? Would it be safer to add a bounds check like
if (nrec > env->prog->len) before the allocation to prevent this
resource churn?

> +
> +	urec_size = attr->cleanup_info_rec_size;
> +	if (urec_size < MIN_BPF_CLEANUP_INFO_SIZE ||
> +	    urec_size > MAX_CLEANUP_INFO_REC_SIZE ||
> +	    urec_size % sizeof(u32)) {
> +		verbose(env, "invalid cleanup info rec size %u\n", urec_size);
> +		return -EINVAL;
> +	}
> +
> +	krecord = kvcalloc(nrec, krec_size, GFP_KERNEL_ACCOUNT | __GFP_NOWARN);

[ ... ]

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