Re: [RFC PATCH bpf-next v1 15/16] selftests/bpf: Test typed arena page allocation and release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: physical pages freed before TLB flush in typed_arena_alloc_pages()
- [High] bpf: bpf_typed_arena_free_pages() frees adjacent chunks due to unaligned addr
--

commit 45f79d175e13ad1f2540fd16c264cce33991c839
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Test typed arena page allocation and release

Allocate a page of typed arena objects, write one, reach it again through
an opaque copy of its address and its neighbour by arithmetic on that copy,
and check that the kfunc receives the registered typed arena in place of
the type ID. Take the chunk a value names and see the same chunk refused
twice, once for the chunk and once for a page inside it. Ask for one page
of an object that spans several and see the request rounded up to the
object's chunk, with the granted count written back. Release a chunk and
see it keep its objects and stay taken within the same invocation, since
the release is deferred behind a grace period.

Reject a use of the returned pointer without a NULL check, a struct without
special fields, and a map that is not the program's arena. Declare the two
kfuncs in bpf_experimental.h.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_typed_arena.c b/tools/testing/selftests/bpf/progs/verifier_typed_arena.c
> index 808a65611d4ef..8ec75fd1f97bf 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_typed_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_typed_arena.c
> @@ -1157,6 +1157,148 @@ int ptr_field_sanitize_on_one_path(void *ctx)
>  	return 0;
>  }
>  
> +#define TYPE_ID(T) bpf_core_type_id_local(T)
> +
> +SEC("syscall")
> +__description("allocated pages hold real objects, reachable through any value that lands in them")
> +__success __retval(0)
> +__xlated("r2 = 0x{{[0-9a-f]+[0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f][0-9a-f]}}")
> +__xlated("call kernel-function")
> +int pages_alloc(void *ctx)
> +{
> +	struct typed_obj *obj, *again, *next;
> +	void *opaque;
> +	__u32 cnt = 1;
> +
> +	obj = bpf_typed_arena_alloc_pages(&arena, TYPE_ID(struct typed_obj), NULL, &cnt,
> +					  NUMA_NO_NODE);

[Severity: Critical]
Is it possible for physical pages to be freed before the TLB flush occurs?

Looking at typed_arena_alloc_pages() in kernel/bpf/arena.c, when an
allocation fails midway:

kernel/bpf/arena.c:typed_arena_alloc_pages() {
    ...
			if (data.i)
				apply_to_existing_page_range(&init_mm, data.start,
							     (unsigned long)data.i << PAGE_SHIFT,
							     apply_range_clear_typed_cb, &cdata);
			free_pages_nolock(head, order);
			goto back_out;
    ...
}

If a concurrent page fault occurs, can this result in the physical pages being
returned to the allocator before the TLB flush in the back_out path?

> +	if (!obj)
> +		return 1;

[ ... ]

> +SEC("syscall")
> +__description("a released chunk keeps its objects and stays taken until the grace period has passed")
> +__success __retval(0)
> +int pages_free(void *ctx)
> +{
> +	struct typed_obj *obj;
> +	__u32 cnt = 1;
> +
> +	obj = bpf_typed_arena_alloc_pages(&arena, TYPE_ID(struct typed_obj), NULL, &cnt,
> +					  NUMA_NO_NODE);
> +	if (!obj)
> +		return 1;
> +	obj->value = 7;
> +	bpf_typed_arena_free_pages(&arena, TYPE_ID(struct typed_obj), obj, 1);

[Severity: High]
Does this code handle unaligned offset values correctly? 

Looking at typed_arena_free_pages() in kernel/bpf/arena.c, when an unaligned
addr is passed, the calculation for the last chunk might spill over into the
next chunk:

kernel/bpf/arena.c:typed_arena_free_pages() {
	off = addr - base;
	if (page_cnt > (size - off) >> PAGE_SHIFT)
		return;
	first = off >> ta->chunk_shift;
	last = (off + ((unsigned long)page_cnt << PAGE_SHIFT) - 1) >> ta->chunk_shift;
    ...
}

Can this lead to unintentional freeing of adjacent chunks?

> +	if (obj->value != 7)
> +		return 2;
> +	cnt = 1;
> +	if (bpf_typed_arena_alloc_pages(&arena, TYPE_ID(struct typed_obj), obj, &cnt, NUMA_NO_NODE))
> +		return 3;
> +	return 0;
> +}

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926233503.3114147-1-memxor@gmail.com?part=15