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 causes trace buffer inflation. --
--- Patch [1]: [PATCH v8 1/4] kallsyms: embed source file:line info in kernel 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 long > address, > len += sprintf(buffer + len, "]"); > } > > + /* > + * 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 += 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 bytes stored. If the source file path is very long and truncation occurs, snprintf() 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() writes the truncated string, the tail of the allocated ring buffer remains uninitialized. Future callers attempting pointer advancement via buf += 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; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
