Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] Test failures on architectures using function descriptors
- [Medium] Silently failing tests due to missing Kconfig dependency for module lineinfo
- [Medium] Test flakiness due to incorrect assumption about compiler optimization and function size
- [Medium] Missing NULL pointer check for test buffer allocation leading to potential kernel panic
--

commit 799168a56f108698328a395b2e257d42cad0b5d4
Author: Sasha Levin [off-list ref]

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.

[ ... ]

[Severity: Medium]
If this test is built as a loadable module and CONFIG_KALLSYMS_LINEINFO_MODULES
is disabled, will the module lineinfo tests fail at runtime? Should this Kconfig
also depend on KALLSYMS_LINEINFO_MODULES when built as a module, or should the
test suite gracefully skip module-specific tests if that configuration is not
enabled?

[ ... ]

+static char *alloc_sym_buf(struct kunit *test)
+{
+	return kunit_kzalloc(test, KSYM_SYMBOL_LEN, GFP_KERNEL);
+}

[Severity: Medium]
Can kunit_kzalloc() return NULL here and cause a kernel panic when callers
unconditionally pass the buffer to sprint_with_lineinfo()? Would it be better
to add a KUNIT_ASSERT_NOT_NULL() check here or validate the return value in
the callers?

[ ... ]

+static noinline int lineinfo_target_many_lines(void)
+{
+	int a = 0;
+
+	barrier();
+	a += 1;
+	a += 2;

[ ... ]

+	a += 10;
+	barrier();
+	return a;
+}

[Severity: Medium]
Is it possible that standard compiler optimizations will fold the arithmetic
in this function away, causing it to compile to fewer than 8 bytes?

If that happens, wouldn't the test_many_lines_mid_function() test below fail
because it adds 8 bytes to the start address and ends up looking outside the
function bounds? Should WRITE_ONCE() or inline assembly be used here to
prevent the arithmetic from being optimized out?

[ ... ]

+static void test_normal_function(struct kunit *test)
+{
+	char *buf = alloc_sym_buf(test);
+	unsigned long addr = (unsigned long)lineinfo_target_normal;

[Severity: Medium]
On architectures that use function descriptors (like PowerPC ELFv1 or ia64),
will casting lineinfo_target_normal directly to unsigned long yield a data
descriptor address instead of the instruction pointer?

Could this cause sprint_backtrace() to fail to resolve the symbol, breaking
the tests unconditionally on those architectures? Does this require using
dereference_symbol_descriptor() to get the actual instruction pointer?

[ ... ]

+static void test_pBb_format(struct kunit *test)
+{
+	char *buf = alloc_sym_buf(test);
+	/*
+	 * %pBb uses sprint_backtrace_build_id which subtracts 1 from the
+	 * address, so pass addr+1 to resolve back to the function.
+	 */
+	void *addr = (void *)((unsigned long)lineinfo_target_normal + 1);

[Severity: Medium]
Similarly, adding 1 to a function descriptor pointer instead of an instruction
pointer creates a corrupt address. Will this cause snprintf() with %pBb to
fail to format the backtrace correctly on architectures using function
descriptors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706142914.2398993-1-sashal@kernel.org?part=4