Re: [PATCH bpf-next 16/36] bpf: save DFS postorder numbers for program instructions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: memory leak of env->cfg.postorder_nums in bpf_prune_dead_branches()
--

commit 3333ceb14d7fa72313c9d88f2f43fc886dbdac3d
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: save DFS postorder numbers for program instructions

Dominator intersections and SCEV worklist scheduling need to compare
instructions by their DFS postorder rank. Save per-instruction
postorder numbers alongside the existing postorder sequence.

> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index b0bd9ba951dfb..bd771efec66a3 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c

[ ... ]

> @@ -774,43 +779,52 @@ int bpf_check_cfg(struct bpf_verifier_env *env)
>   */
>  int bpf_compute_postorder(struct bpf_verifier_env *env)
>  {
> -	u32 cur_postorder, i, top, stack_sz, s;
> -	int *stack = NULL, *postorder = NULL, *state = NULL;
> -	struct bpf_iarray *succ;
> +	int *stack = NULL, *postorder = NULL, *postorder_nums = NULL;
> +	int subprog_idx, stack_sz, cur, s, cur_postorder, start;
> +	struct dfs_state *state = NULL;
> +	struct bpf_iarray *succ = NULL;
>  
> +	postorder_nums = kvzalloc_objs(int, env->prog->len, GFP_KERNEL_ACCOUNT);

[Severity: High]
Does this code leak memory when bpf_compute_postorder() is called multiple
times?

If a user loads a BPF program that contains a dead branch determinable at load
time, it can cause bpf_prune_dead_branches() to successfully fold the branch
and call bpf_compute_postorder() a second time:

kernel/bpf/const_fold.c:bpf_prune_dead_branches() {
    ...
    kvfree(env->cfg.insn_postorder);
    env->cfg.insn_postorder = NULL;
    return bpf_compute_postorder(env);
}

The array allocated here unconditionally overwrites env->cfg.postorder_nums
without freeing the previously allocated one. Can we add a call to
kvfree(env->cfg.postorder_nums) in bpf_prune_dead_branches() to prevent a leak
of up to 4MB per program load?

[ ... ]

> -	env->subprog_info[i].postorder_start = cur_postorder;
> +	env->subprog_info[subprog_idx].postorder_start = cur_postorder;
> +	env->cfg.postorder_nums = postorder_nums;
>  	env->cfg.insn_postorder = postorder;
>  	env->cfg.cur_postorder = cur_postorder;
>  	kvfree(stack);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926-scev-minimal-rebase-v1-0-c8e5ab5ba79f@gmail.com?part=16