Re: [PATCH v3 bpf-next 6/7] selftests/bpf: Test BTF sysfs inline representations
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-03 19:54:59
Also in:
bpf
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] bpf: module_btf memory leak when .inline sysfs entry is missing
- [Low] bpf: missing continue statement after unexpected stat() failure leads to cascading failures
- [Low] bpf: partial memory leak on unexpected mmap success in btf sysfs test
--
commit de628b5a108fca7fc52dbae8ef2aa0fc5fbd85bd
Author: Alan Maguire [off-list ref]
selftests/bpf: Test BTF sysfs inline representations
This commit tests BTF sysfs inline representations by loading vmlinux,
vmlinux.inline, and all module/module.inline entries in /sys/fs/btf. This
verifies the kernel and module inline representations and sanity-checks
relocations.
There is a minor memory leak on an error path in test_btf_mmap_sysfs()
when the mmap with an invalid size unexpectedly succeeds:
tools/testing/selftests/bpf/prog_tests/btf_sysfs.c:test_btf_mmap_sysfs() {
...
raw_data = mmap(NULL, end + 1, PROT_READ, MAP_PRIVATE, fd, 0);
if (!ASSERT_EQ(raw_data, MAP_FAILED, "mmap_btf_invalid_size"))
goto cleanup;
...
cleanup:
btf__free(btf);
if (raw_data && raw_data != MAP_FAILED)
munmap(raw_data, btf_size);
...
}
[Severity: Low]
Does this leak the final page allocated by the end + 1 size?
If the oversized mmap unexpectedly succeeds, it allocates an extra page, but
the cleanup label only unmaps up to btf_size.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c b/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c index 3923e64c4c1d0..069c4483b3a21 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_sysfs.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -75,7 +80,78 @@ static void test_btf_mmap_sysfs(const char *path, struct btf *base) close(fd); } +static void test_btf_inline_sysfs_all(void) +{
[ ... ]
quoted hunk ↗ jump to hunk
+ if (!base_btf) { + module_btf = btf__parse_split(btf_path, vmlinux_btf); + err = libbpf_get_error(module_btf); + if (err) { + /* A module can be unloaded while its sysfs entry is iterated. */ + if (err == -ENOENT) + continue; + ASSERT_OK(err, "parse_module_btf"); + continue; + } + base_btf = module_btf; + } + if (stat(inline_path, &st)) { + err = errno; + if (err == ENOENT) + continue;
[Severity: Medium] Does this early continue leak the memory allocated for module_btf? When an .inline file does not exist, the continue statement bypasses the btf__free(module_btf) call at the bottom of the loop. Since most modules might lack this representation, this could leak memory for every module.
+ ASSERT_OK(err, "stat_inline_btf"); + }
[Severity: Low] Should there be a continue statement here to avoid cascading failures? If stat() fails with an error other than ENOENT, the code asserts but falls through to parse the nonexistent or inaccessible inline file instead of jumping to the next iteration.
quoted hunk ↗ jump to hunk
+ if (base_btf == vmlinux_btf) + test_btf_mmap_sysfs(inline_path, base_btf); + inline_btf = btf__parse_split(inline_path, base_btf); + err = libbpf_get_error(inline_btf); + if (!err) + btf__free(inline_btf); + ASSERT_OK(err, "parse_inline_btf"); + btf__free(module_btf); + } + closedir(dir); + + btf__free(vmlinux_btf); +}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20261003194323.817885-1-alan.maguire@oracle.com?part=6