Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [Medium] The KUnit test `test_sprint_symbol_format` unconditionally fails because it mistakenly expects `+0x` for a zero offset. Pre-existing issues: - [High] kallsyms_lookup_buildid() can return negative error codes like -E2BIG, leading to a stack buffer underflow in __sprint_symbol(). -- --- Patch [4]: [PATCH v8 4/4] kallsyms: add KUnit tests for lineinfo feature --- commit 337c29ff08b7b23d898a00240203617a93c2c906 Author: Sasha Levin <[email protected]> kallsyms: add KUnit tests for lineinfo feature Add a KUnit test module (CONFIG_LINEINFO_KUNIT_TEST) that verifies the kallsyms lineinfo feature produces correct source file:line annotations in stack traces. > diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c > index b5eff2ba8e5d7..b0475f1e7f791 100644 > --- a/kernel/kallsyms.c > +++ b/kernel/kallsyms.c [Severity: High] This is a pre-existing issue, but could this lead to a stack buffer underflow? In kernel/kallsyms.c:__sprint_symbol(): len = kallsyms_lookup_buildid(address, &size, &offset, &modname, &buildid, buffer); if (!len) return sprintf(buffer, "0x%lx", address - symbol_offset); ... if (add_offset) len += sprintf(buffer + len, "+%#lx/%#lx", offset, size); If a loadable module or BPF program contains a symbol name that exceeds KSYM_NAME_LEN, functions like module_address_lookup() return -E2BIG (-7). This negative value propagates up through kallsyms_lookup_buildid() and bypasses the 'if (!len)' check. Would 'buffer + len' then point before the allocated buffer, causing an out-of-bounds write during the subsequent sprintf? [ ... ] > diff --git a/lib/tests/lineinfo_kunit.c b/lib/tests/lineinfo_kunit.c > new file mode 100644 > index 0000000000000..2e66e8ebb3124 > --- /dev/null > +++ b/lib/tests/lineinfo_kunit.c > @@ -0,0 +1,1040 @@ [ ... ] > +static void test_sprint_symbol_format(struct kunit *test) > +{ > + char *buf = alloc_sym_buf(test); > + unsigned long addr = target_addr(lineinfo_target_normal); > + > + sprint_symbol(buf, addr); > + > + /* Should contain +0x and /0x for offset/size */ > + KUNIT_EXPECT_NOT_NULL_MSG(test, strstr(buf, "+0x"), > + "Missing offset in: %s", buf); [Severity: Medium] Will this assertion fail unconditionally due to standard C formatting rules? Since test_sprint_symbol_format() passes the exact start address of lineinfo_target_normal, the resulting offset is 0. When __sprint_symbol() formats a zero offset using '+%#lx/%#lx', the '%#lx' specifier omits the '0x' prefix for a zero value, resulting in '+0/' instead of '+0x0/'. Does this cause strstr(buf, "+0x") to always return NULL and break the test? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
