Re: [PATCHv2 bpf-next 5/8] selftests/bpf: Test user memory mmap kfuncs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] selftests/bpf: call to undefined get_procmap_info() in user_mem.c
- [Medium] selftests/bpf: unresolved kfunc externs in progs/user_mem.c
--

commit c7cc5f98b3a4b4d8d168ef0934e6c41fe197849d
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Test user memory mmap kfuncs

This commit adds BPF selftests for bpf_user_anon_mmap and bpf_user_munmap
invoked from a sleepable uprobe, verifying mapping permissions and
unmapping behavior.

> diff --git a/tools/testing/selftests/bpf/prog_tests/user_mem.c b/tools/testing/selftests/bpf/prog_tests/user_mem.c
> new file mode 100644
> index 0000000000000..f301a18437062
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/user_mem.c
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include <sys/mman.h>
> +#include <sys/personality.h>
> +#include <unistd.h>
> +
> +#include "user_mem.skel.h"
> +#include "trace_helpers.h"
> +
> +enum {
> +	USER_MEM_MMAP = 1,
> +	USER_MEM_UNMAP,
> +};
> +
> +static noinline void user_mem_trigger(void)
> +{
> +	asm volatile ("" ::: "memory");
> +}
> +
> +static bool check_user_mem_vma(void *addr, unsigned long size, const char *perms)
> +{
> +	size_t start, end;
> +	char vma_perms[5];
> +	int err;
> +
> +	err = get_procmap_info(addr, &start, &end, vma_perms);
> +	if (!ASSERT_OK(err, "user_mem_vma"))
> +		return false;
> +
> +	/* VMA can be merged, so ensure it covers at least the requested range. */
> +	if (!ASSERT_GE(end - (size_t)addr, size, "vma_size"))
> +		return false;
> +
> +	return !perms || ASSERT_STREQ(vma_perms, perms, "vma_perms");
> +}
> +
> +static bool check_user_mem_unmapped(void *addr)
> +{
> +	size_t start, end;
> +	int err;
> +
> +	err = get_procmap_info(addr, &start, &end, NULL);

[Severity: Medium]
Where is get_procmap_info() defined? Both check_user_mem_vma() and
check_user_mem_unmapped() call get_procmap_info(), and this file includes
trace_helpers.h, but neither tools/testing/selftests/bpf/trace_helpers.h nor
tools/testing/selftests/bpf/trace_helpers.c declares or defines
get_procmap_info().

Because test_progs objects are compiled with -Wall -Werror, won't calling an
undeclared and undefined get_procmap_info() cause compiling and linking
tools/testing/selftests/bpf to fail?

> diff --git a/tools/testing/selftests/bpf/progs/user_mem.c b/tools/testing/selftests/bpf/progs/user_mem.c
> new file mode 100644
> index 0000000000000..e3b0689f5f2a5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/user_mem.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <vmlinux.h>
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +
> +#include "bpf_misc.h"
> +
> +char _license[] SEC("license") = "GPL";
> +
> +extern unsigned long bpf_user_anon_mmap(unsigned long len, unsigned long prot) __ksym;
> +extern int bpf_user_munmap(unsigned long addr, unsigned long len) __ksym;

[ ... ]

> +SEC("uprobe.s//proc/self/exe:user_mem_trigger")
> +int user_mem_test(struct pt_regs *ctx)
> +{
> +	if ((bpf_get_current_pid_tgid() >> 32) != monitored_pid)
> +		return 0;
> +
> +	switch (action) {
> +	case USER_MEM_MMAP:
> +		user_addr = bpf_user_anon_mmap(map_len, map_prot);
> +		break;
> +	case USER_MEM_UNMAP:
> +		ret = bpf_user_munmap(user_addr, map_len);
> +		break;
> +	}

[Severity: Medium]
Can user_mem__open_and_load() in test_user_mem() succeed when loading this
program?

user_mem_test() calls bpf_user_anon_mmap() and bpf_user_munmap(), which are
declared as required __ksym externs, but neither symbol is defined or
registered as a BPF kfunc in the kernel or bpf_testmod. Won't libbpf fail to
resolve these kfunc BTF IDs during user_mem__open_and_load(), causing
test_user_mem() to fail unconditionally?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103075916.100050-6-jolsa@kernel.org?part=1