Re: [PATCH v8 1/4] kallsyms: embed source file:line info in kernel stack traces

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] Use of `snprintf` violates `sprint_symbol` API contract and caus=
es trace buffer inflation.
--

--- Patch [1]: [PATCH v8 1/4] kallsyms: embed source file:line info in kern=
el stack traces ---
commit 8a14a0edefef69e5b032ac47662777d287f71164
Author: Sasha Levin <[email protected]>

kallsyms: embed source file:line info in kernel stack traces

Add CONFIG_KALLSYMS_LINEINFO, which embeds a compact address-to-line
lookup table in the kernel image so stack traces directly print source
file and line number information.

> diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
> --- a/kernel/kallsyms.c
> +++ b/kernel/kallsyms.c

[ ... ]

> @@ -497,6 +567,23 @@ static int __sprint_symbol(char *buffer, unsigned lo=
ng address,
>  		len +=3D sprintf(buffer + len, "]");
>  	}
> =20
> +	/*
> +	 * Append "(file:line)" only for stack-backtrace consumers.  Plain
> +	 * sprint_symbol() backs %ps, and many existing format strings tack
> +	 * literal "()" after %ps to indicate a function call ("foo()
> +	 * replaced with bar()"); appending lineinfo there would produce a
> +	 * confusing "foo (file:line)()".
> +	 */
> +	if (add_lineinfo && IS_ENABLED(CONFIG_KALLSYMS_LINEINFO) && !modname) {
> +		const char *li_file;
> +		unsigned int li_line;
> +
> +		if (kallsyms_lookup_lineinfo(address, sym_start,
> +					     &li_file, &li_line))
> +			len +=3D snprintf(buffer + len, KSYM_SYMBOL_LEN - len,
> +					" (%s:%u)", li_file, li_line);

[Severity: Medium]
Is it safe to use snprintf() here instead of scnprintf()?

The API contract for sprint_symbol() specifies it returns the number of byt=
es
stored. If the source file path is very long and truncation occurs, snprint=
f()
will return the un-truncated length.

This can cause caller logic errors. For example, if a kprobe triggers on a
symbol with a long source path, fetch_store_symstrlen() will allocate buffer
space based on this inflated return value. When fetch_store_symstring() wri=
tes
the truncated string, the tail of the allocated ring buffer remains
uninitialized. Future callers attempting pointer advancement via
buf +=3D sprint_symbol(...) could also be exposed to out-of-bounds writes.

Could this be changed to use scnprintf() to ensure it returns the actual
number of bytes written?

> +	}
> +
>  	return len;
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260801143219.3975=
[email protected]?part=3D1