Re: [PATCH bpf-next v2 18/43] bpf: compute immediate dominators

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: integer overflow in compute_predecessors size calculation
--

commit ce110737451057ca4a576ab4cd5332b75722cc64
Author: Eduard Zingerman <eddyz87@gmail.com>
bpf: compute immediate dominators

This commit introduces a new immediate dominator tree computation for the
BPF verifier. It analyzes each subprogram to find exit conditions that
dominate loop backedges.

> diff --git a/kernel/bpf/loops.c b/kernel/bpf/loops.c
> new file mode 100644
> index 0000000000000..0a9e10edca871
> --- /dev/null
> +++ b/kernel/bpf/loops.c
> @@ -0,0 +1,156 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2026 Meta Platforms, Inc. and affiliates. */
> +
> +#include <linux/slab.h>
> +#include <linux/sched/signal.h>
> +#include <linux/bpf_verifier.h>
> +
> +static struct bpf_iarray **compute_predecessors(struct bpf_verifier_env *env)
> +{
> +	struct bpf_iarray *succ, *preds, **result;
> +	struct bpf_prog *prog = env->prog;
> +	u32 *num_preds, i, s, sz, len = prog->len;

[Severity: Critical]
Since sz is typed as a 32-bit integer, could it overflow during the size
calculations below?

> +	struct bpf_insn *insn;
> +	void *tmp;
> +
> +	num_preds = kvcalloc(prog->len, sizeof(u32), GFP_KERNEL_ACCOUNT);
> +	if (!num_preds)
> +		return NULL;
> +
> +	/*
> +	 * 'result' layout:
> +	 *  - array of pointers (struct bpf_iarray *)[len]
> +	 *  - struct bpf_iarray one after another
> +	 */
> +	sz = sizeof(struct bpf_iarray) * len;
> +	sz += sizeof(struct bpf_iarray *) * len;
> +	for (i = 0; i < len; i++) {
> +		insn = env->prog->insnsi + i;
> +		succ = bpf_insn_successors(env, i);
> +		sz += sizeof(u32) * succ->cnt;

[Severity: Critical]
Because sz is accumulated here without overflow checks, can a BPF program
with many gotox instructions sharing a large jump table repeatedly add a large
succ->cnt, causing sz to wrap around?

> +		iarray_for_each(s, succ) {
> +			num_preds[s]++;
> +		}
> +		if (bpf_is_ldimm64(insn))
> +			i++;
> +	}
> +
> +	result = kvzalloc(sz, GFP_KERNEL_ACCOUNT);
> +	if (!result) {
> +		kvfree(num_preds);
> +		return NULL;
> +	}
> +
> +	tmp = (void *)&result[len];
> +	for (i = 0; i < len; i++) {
> +		result[i] = tmp;

[Severity: Critical]
If sz wraps around and a small buffer is allocated, does tmp end up pointing
completely out-of-bounds?

Would this loop and subsequent array accesses then overwrite arbitrary kernel
heap memory, potentially leading to critical memory corruption when an
unprivileged user loads a crafted BPF program?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-scev-minimal-rebase-v2-0-f6254e29fdf2@gmail.com?part=18