Re: [PATCH bpf-next v2 1/4] bpf: arena: populate vm_area without allocating memory

[Puranjay Mohan <puranjay@kernel.org>]

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Nov 14, 2025 at 6:57 AM Puranjay Mohan <puranjay@kernel.org> wrote:
>>
>> bot+bpf-ci@kernel.org writes:
>>
>> >> diff --git a/kernel/bpf/arena.c b/kernel/bpf/arena.c
>> >> index 1074ac445..48b8ffba3 100644
>> >> --- a/kernel/bpf/arena.c
>> >> +++ b/kernel/bpf/arena.c
>> >
>> > [ ... ]
>> >
>> >> @@ -92,6 +93,62 @@ static long compute_pgoff(struct bpf_arena *arena, long uaddr)
>> >>      return (u32)(uaddr - (u32)arena->user_vm_start) >> PAGE_SHIFT;
>> >>  }
>> >>
>> >> +struct apply_range_data {
>> >> +    struct page **pages;
>> >> +    int i;
>> >> +};
>> >> +
>> >> +static int apply_range_set_cb(pte_t *pte, unsigned long addr, void *data)
>> >> +{
>> >> +    struct apply_range_data *d = data;
>> >> +    struct page *page;
>> >> +
>> >> +    if (!data)
>> >> +            return 0;
>> >> +    /* sanity check */
>> >> +    if (unlikely(!pte_none(ptep_get(pte))))
>> >> +            return -EBUSY;
>> >> +
>> >> +    page = d->pages[d->i++];
>> >> +    /* paranoia, similar to vmap_pages_pte_range() */
>> >> +    if (WARN_ON_ONCE(!pfn_valid(page_to_pfn(page))))
>> >> +            return -EINVAL;
>> >> +
>> >> +    set_pte_at(&init_mm, addr, pte, mk_pte(page, PAGE_KERNEL));
>> >> +    return 0;
>> >> +}
>> >> +
>> >> +static int apply_range_clear_cb(pte_t *pte, unsigned long addr, void *data)
>> >> +{
>> >> +    pte_t old_pte;
>> >> +    struct page *page;
>> >> +
>> >> +    /* sanity check */
>> >> +    old_pte = ptep_get(pte);
>> >> +    if (pte_none(old_pte) || !pte_present(old_pte))
>> >> +            return 0; /* nothing to do */
>> >> +
>> >> +    /* get page and free it */
>> >> +    page = pte_page(old_pte);
>> >> +    if (WARN_ON_ONCE(!page))
>> >> +            return -EINVAL;
>> >> +
>> >> +    pte_clear(&init_mm, addr, pte);
>> >> +
>> >> +    /* ensure no stale TLB entries */
>> >> +    flush_tlb_kernel_range(addr, addr + PAGE_SIZE);
>> >> +
>> >> +    __free_page(page);
>> >> +
>> >> +    return 0;
>> >> +}
>> >> +
>> >> +static int populate_pgtable_except_pte(struct bpf_arena *arena)
>> >> +{
>> >> +    return apply_to_page_range(&init_mm, bpf_arena_get_kern_vm_start(arena),
>> >> +                               KERN_VM_SZ - GUARD_SZ, apply_range_set_cb, NULL);
>> >> +}
>> >> +
>> >>  static struct bpf_map *arena_map_alloc(union bpf_attr *attr)
>> >>  {
>> >>      struct vm_struct *kern_vm;
>> >
>> > [ ... ]
>> >
>> >> @@ -144,6 +201,11 @@ static struct bpf_map *arena_map_alloc(union bpf_attr *attr)
>> >>              goto err;
>> >>      }
>> >>      mutex_init(&arena->lock);
>> >> +    err = populate_pgtable_except_pte(arena);
>> >> +    if (err) {
>> >> +            bpf_map_area_free(arena);
>> >> +            goto err;
>> >> +    }
>> >                      ^^^^
>> >
>> > Can this leak the range tree? Looking at the code, range_tree_set() was
>> > called earlier in this function and can allocate range_node structures
>> > via range_tree_clear()->kmalloc_nolock(). If populate_pgtable_except_pte()
>> > fails here, the error path calls bpf_map_area_free(arena) but doesn't call
>> > range_tree_destroy(&arena->rt) first. Compare with arena_map_free() which
>> > always calls range_tree_destroy() before freeing the arena.
>>
>> As the range tree is empty at this point, we can be sure that
>> range_tree_clear() in range_tree_set() will not allocate anything.
>
> range_tree_clear() won't clear anything, but AI pointed in
> the right direction.
> Look at what range_tree_set() does. It will allocate for sure.

If I am understanding it correctly, range_tree_set() allocates memory
using kmalloc_nolock() and it fails when this allocation fails, so in
the error path we don't need to do anything as no allocation was successful.