Re: [PATCH v4 bpf-next 7/7] selftests/bpf: Add a test verifying inline information
flat view
From: sashiko-bot@kernel.org
Date: 2026-10-09 12:33:54
Also in:
bpf
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] selftests/bpf: module_get_base_addr() desyncs on trailing /proc/modules tokens - [Medium] bpf: btf_inline test incorrectly allows memory-spilled parameter -- commit 4adb006744e35941ca2b357431675f3202d1100f Author: Alan Maguire [off-list ref] Subject: selftests/bpf: Add a test verifying inline information This commit adds a selftest to verify inline information for bpf_testmod. It checks that an always-inlined function has a single location parameter encoded in a register, and that the LOCSEC descriptor offset is within the expected range.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/prog_tests/btf_inline.c b/tools/testing/selftests/bpf/prog_tests/btf_inline.c new file mode 100644 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/btf_inline.c
[ ... ]
quoted hunk ↗ jump to hunk
+ lp = btf_loc_param(t); + if (!ASSERT_TRUE(lp->flags & BTF_LOC_PARAM_REG, "param_has_reg")) + goto out;
[Severity: Medium] Because the variable in testmod_inline_len() below is forced into memory, its BTF location is encoded as a register-relative memory offset, which sets both BTF_LOC_PARAM_REG and BTF_LOC_PARAM_OFFSET. Does this bitwise AND accidentally pass for memory offsets since they have a base register? If so, this check might be masking the fact that the parameter was actually spilled to the stack instead of being purely encoded in a register as the commit message describes. [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c index 93847ca6293b4..b4f0af2aeca45 100644 --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -735,6 +735,13 @@ bpf_testmod_test_read(struct file *file, struct kobject *kobj, EXPORT_SYMBOL(bpf_testmod_test_read); ALLOW_ERROR_INJECTION(bpf_testmod_test_read, ERRNO); +static __always_inline size_t testmod_inline_len(size_t len) +{ + /* Keep an instruction in the inline site without changing len. */ + asm volatile("" : "+m"(len));
[Severity: Medium] Does the "+m" constraint here force the len variable into memory rather than a register? The commit message explicitly states the intent is to verify a parameter "encoded in a register", but this constraint forces a stack spill instead.
+ return len; +}
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c index 679008b310d9a..f3a1d627699da 100644 --- a/tools/testing/selftests/bpf/trace_helpers.c +++ b/tools/testing/selftests/bpf/trace_helpers.c
[ ... ]
quoted hunk ↗ jump to hunk
+unsigned long long module_get_base_addr(const char *module) +{ + unsigned long long addr, base_addr = 0; + char name[500]; + FILE *f; + + f = fopen("/proc/modules", "r"); + if (!f) + return 0; + + while (fscanf(f, "%499s %*s %*s %*s %*s 0x%llx", name, &addr) == 2) {
[Severity: Medium] Will this loop terminate prematurely if it encounters trailing data on a line in /proc/modules? If a module (like an out-of-tree or unsigned module) has an optional trailing taint string such as "(O)", this fscanf format lacks a line consumer (like %*[^\n]\n) to read to the end of the line. On the next iteration, the %499s token will mistakenly consume the leftover taint string instead of the next module's name, shifting all subsequent fields by one token. When the 0x%llx field then attempts to parse the state string (e.g., "Live"), it will fail to match the "0x" prefix and return 1, causing the loop to exit early and return 0 instead of the correct base address.
quoted hunk ↗ jump to hunk
+ if (strcmp(module, name) == 0) { + base_addr = addr; + break; + } + } + fclose(f); + return base_addr; +}
-- Sashiko AI review · https://sashiko.dev/#/patchset/20261009112324.1442319-1-alan.maguire@oracle.com?part=7