Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Unbounded bump-allocation of `bpf_verifier_stack_elem` causes severe 
transient memory bloat during BPF verification.
--

--- Patch [3]: [PATCH 3/9] bpf/verifier: Route verifier stack state node 
allocations to folio_pool ---
commit 1f1ee699bf3c9e5a960869e7d6dbd7c57b050ee5
Author: Jim Cromie <[email protected]>

bpf/verifier: Route verifier stack state node allocations to folio_pool

This commit embeds a folio_pool inside bpf_verifier_env to allocate transient
bpf_verifier_stack_elem frames, releasing them in bulk at the end of
bpf_check().

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1725,7 +1725,7 @@ static int pop_stack(struct bpf_verifier_env *env, int 
> *prev_insn_idx,
>               *prev_insn_idx = head->prev_insn_idx;
>       elem = head->next;
>       bpf_free_verifier_state(&head->st, false);
> -     kfree(head);
> +     folio_pool_free_elem(head);

[Severity: High]
Does this cause unbounded memory growth during verification?

Looking at include/linux/folio_pool.h, folio_pool_free_elem() appears to be a
no-op for pointers allocated from the pool:

static inline void folio_pool_free_elem(const void *ptr)
{
        if (!ptr || is_folio_pool_ptr(ptr))
                return;
        kvfree(ptr);
}

Since elements popped from the verifier stack are not actually freed, won't
the memory consumption scale with the total number of branches explored
rather than the maximum depth of the verification stack?

>       env->head = elem;
>       env->stack_size--;
>       return 0;

[ ... ]

> @@ -1751,7 +1753,9 @@ static struct bpf_verifier_state *push_stack(struct 
> bpf_verifier_env *env,
>       struct bpf_verifier_stack_elem *elem;
>       int err;
>  
> -     elem = kzalloc_obj(struct bpf_verifier_stack_elem, GFP_KERNEL_ACCOUNT);
> +     elem = folio_pool_alloc_obj(env, state_pool,
> +                                 struct bpf_verifier_stack_elem,
> +                                 GFP_KERNEL_ACCOUNT);

[Severity: High]
With every explored branch allocating a new stack element here, and
pop_stack() abandoning the memory, can this transient memory bloat lead to
memory exhaustion and potential OOMs when verifying complex BPF programs
with many branches?

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=3

Reply via email to