Re: [PATCH v8 4/4] kallsyms: add KUnit tests for lineinfo feature

[sashiko-bot@kernel.org]

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 <sashal@kernel.org>

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/20260801143219.3975824-1-sashal@kernel.org?part=4