Re: [PATCH 2/2] selftests/bpf: Add test validating arena VMA tracking across fork

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The test bypasses the intended VMA tracking logic because arena mmaps are created with VM_DONTCOPY, preventing the child from inheriting the VMA.
- [High] Double `munmap()` of libbpf-managed memory leads to potential silent memory corruption in concurrent test environments.
- [Medium] Flaky reliance on `usleep()` for RCU grace period synchronization.
--

commit ee7a95885f283c6cd8d8ee4195aa3d4bcee4e6a6
Author: Daniel Hodges <git@danielhodges.dev>

selftests/bpf: Add test validating arena VMA tracking across fork
    
This commit adds a BPF selftest to validate that freeing arena pages does
not trigger a use-after-free when a forked child holds an inherited arena mmap.

> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_fork_free.c b/tools/testing/selftests/bpf/prog_tests/arena_fork_free.c
> new file mode 100644
> index 0000000000000..6a6e1d4197cf9
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/arena_fork_free.c

[ ... ]

> +	arena_area = bpf_map__initial_value(skel->maps.arena, &arena_sz);
> +	if (!ASSERT_OK_PTR(arena_area, "arena_area"))
> +		goto out;
> +	arena_sz = bpf_map__max_entries(skel->maps.arena) * PAGE_SIZE;
> +
> +	if (!ASSERT_OK(pipe(pipe_fds), "pipe"))
> +		goto out;
> +
> +	/* Step 2: fork — child inherits arena mmap */
> +	child = fork();

Does the child actually inherit the arena mmap here? 

The kernel's arena_map_mmap() sets VM_DONTCOPY on the arena VMA, meaning the
child process will not inherit it during fork() and arena_vm_open() will not
be called.

This causes the parent's munmap() to leave the tracking list empty, so when
bpf_arena_free_pages() triggers zap_pages(), it iterates over an empty list.
The test seems to pass without exercising the intended VMA tracking logic.

Could the test clear VM_DONTCOPY via madvise(..., MADV_DOFORK) before
calling fork()?

[ ... ]

> +	/* Parent continues */
> +	close(pipe_fds[0]);
> +	pipe_fds[0] = -1;
> +
> +	/* Step 3: munmap the arena in the parent */
> +	ret = munmap(arena_area, arena_sz);

Can this munmap() lead to a double munmap and silent memory corruption?

Because arena_area was obtained from bpf_map__initial_value(), it points to
libbpf's internal map->mmaped pointer. When the test completes and calls
arena_fork_free__destroy(skel), libbpf will unconditionally call munmap()
on this pointer again.

If another thread or parallel test mmaps memory in that address window
between this munmap() and the skeleton destruction, the second munmap()
will silently unmap the unrelated memory.

Would it be better for the test to mmap() the arena independently using its
FD instead of unmapping libbpf's internal pointer?

[ ... ]

> +	/*
> +	 * Step 4: free arena pages via BPF.
> +	 *
> +	 * Wait for the RCU grace period so the parent's VMA slab memory
> +	 * is actually freed (VMA freeing is deferred via call_rcu).
> +	 * This ensures KASAN can detect any stale VMA dereference in
> +	 * zap_pages().
> +	 */
> +	usleep(200000);

Is relying on a fixed timeout reliable for ensuring an RCU grace period
has elapsed?

Under heavy load in CI environments, the grace period might be delayed,
causing KASAN to miss the intended use-after-free because the VMA slab
memory hasn't actually been freed yet.

Could the test use kern_sync_rcu() from the BPF selftests framework to
deterministically wait for the grace period?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260513191322.21319-1-git@danielhodges.dev?part=2