Re: [PATCH v6 8/9] selftests/verification: Add tlob selftests
From: sashiko-bot@kernel.org
Date: 2026-08-20 16:56:33
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Using `$(nproc) - 1` to determine a target CPU ID assumes that CPU IDs are contiguous and start from 0, which breaks on sparse affinity masks or offline CPUs. - [Medium] The ELF parser in `tlob_sym.c` lacks sufficient bounds checks when validating headers and iterating over sections, leading to out-of-bounds memory accesses. --
--- Patch [8]: [PATCH v6 8/9] selftests/verification: Add tlob selftests ---commit cb49807af49abc34cb32dee7d7e1d06575cff80a Author: Wen Yang [off-list ref] selftests/verification: Add tlob selftests This commit adds seven ftrace-style test scripts for the tlob RV monitor, covering uprobe binding management, budget violation detection, and per-state time accounting. It also includes a standalone runner and C helper programs for target execution and ELF symbol parsing.
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/verification/test.d/tlob/uprobe_detail_waiting.tc b/tools/testing/selftests/verification/test.d/tlob/uprobe_detail_waiting.tc new file mode 100644 index 0000000000000..798a04012b6ea --- /dev/null +++ b/tools/testing/selftests/verification/test.d/tlob/uprobe_detail_waiting.tc
[ ... ]
+start_offset=$("$TLOB_SYM" sym_offset "$UPROBE_TARGET" tlob_preempt_work 2>/dev/null)
+stop_offset=$("$TLOB_SYM" sym_offset "$UPROBE_TARGET" tlob_preempt_work_done 2>/dev/null)
+
+# Pick the last CPU to avoid cpu0 which is used by vng infrastructure.
+cpu=$(($(nproc) - 1))
+[Severity: Medium] Does this CPU ID calculation safely handle systems with sparse CPU affinity masks or offline CPUs? If a system has non-contiguous CPU IDs (for example, if CPU 1 is offline but CPUs 0 and 2 are online), nproc will evaluate to 2. This calculation would yield CPU 1, which might cause the subsequent taskset commands to fail with an invalid argument error since CPU 1 is unavailable.
+echo 1 > ../events/rv/detail_env_tlob/enable +echo 1 > ../tracing_on +echo 1 > monitors/tlob/enable +echo > ../trace
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/tools/testing/selftests/verification/tlob_sym.c b/tools/testing/selftests/verification/tlob_sym.c new file mode 100644 index 0000000000000..2d9561331d2f9 --- /dev/null +++ b/tools/testing/selftests/verification/tlob_sym.c
[ ... ]
+ for (int pass = 0; pass < 2 && !found; pass++) {
+ const char *target = pass ? ".dynsym" : ".symtab";
+
+ for (si = 0; si < ehdr->e_shnum && !found; si++) {
+ Elf64_Shdr *sh = &shdrs[si];
+ const char *name = shstrtab + sh->sh_name;[Severity: Medium] Is there a risk of an out-of-bounds read when accessing the string table here? It appears sh->sh_name is used as an offset into shstrtab and passed to strcmp() without bounds checking against the actual string table size. Could a malformed binary cause a segmentation fault when the tool attempts to read beyond the mapped memory?
+ + if (strcmp(name, target) != 0) + continue; + + Elf64_Shdr *strtab_sh = &shdrs[sh->sh_link];
[Severity: Medium] Could this array indexing access memory outside the section header table? The sh->sh_link field is used directly as an index into shdrs. Since there is no verification that sh->sh_link is less than ehdr->e_shnum, a malformed ELF file could cause the tool to read arbitrary memory when mapping headers.
+ const char *strtab = (char *)map + strtab_sh->sh_offset; + Elf64_Sym *syms = (Elf64_Sym *)((char *)map + sh->sh_offset); + uint64_t nsyms = sh->sh_size / sizeof(Elf64_Sym); + uint64_t j;
[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/cover.1787243842.git.wen.yang@linux.dev?part=8