Re: [PATCH v7 2/4] kallsyms: extend lineinfo to loadable modules
From: sashiko-bot@kernel.org
Date: 2026-07-09 16:50:31
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Unvalidated module blob offsets cause unaligned memory accesses, potentially triggering alignment faults in critical contexts. - [Medium] Integer overflow in the `apply_one_dl_reloc` bounds check allows out-of-bounds heap access in the `gen_lineinfo` tool. - [Low] Dynamically allocated memory for `extra_sections` is never freed in the `gen_lineinfo` tool. --
--- Patch [2]: [PATCH v7 2/4] kallsyms: extend lineinfo to loadable modules ---commit be00704ef1eba0bdfe61160ba94617348dfdc7dc Author: Sasha Levin [off-list ref] kallsyms: extend lineinfo to loadable modules This patch extends the CONFIG_KALLSYMS_LINEINFO feature to loadable kernel modules. It uses scripts/gen-mod-lineinfo.sh to generate .mod_lineinfo sections containing binary tables of source file and line number mappings. At runtime, module_lookup_lineinfo() searches these read-only tables to provide NMI-safe file and line annotations for module stack traces.
quoted hunk ↗ jump to hunk
diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c index 0fc11e45df9b9..4888ce8a7f951 100644 --- a/kernel/module/kallsyms.c +++ b/kernel/module/kallsyms.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -494,3 +494,201 @@ int module_kallsyms_on_each_symbol(const char *modname, +static bool module_lookup_lineinfo_section(const void *blob, u32 blob_size, + u32 hdr_offset, + unsigned int section_offset, + unsigned int min_offset, + const char **file, + unsigned int *line) +{ + const struct mod_lineinfo_header *hdr; + const u8 *base; + const u32 *addrs, *lines, *file_offsets; + const u16 *file_ids; + const char *filenames; + u32 num_entries, num_files, filenames_size; + unsigned int low, high, mid; + u16 file_id; + + if (hdr_offset > blob_size || + blob_size - hdr_offset < sizeof(*hdr)) + return false; + + base = (const u8 *)blob + hdr_offset; + hdr = (const struct mod_lineinfo_header *)base;
[Severity: High] Does this code safely handle unaligned memory accesses? The hdr_offset value is read directly from the binary blob and is never validated for 4-byte alignment before being cast to struct mod_lineinfo_header * and subsequently u32 * for the internal arrays. If a malformed or corrupted .ko file provides an unaligned hdr_offset, dereferencing these pointers could trigger a hardware alignment trap on strict-alignment architectures. Since this code is designed to be called during NMI or panic stack traces, taking an alignment fault here would cause a recursive fault and hang the system without producing the crash dump. Could we add an alignment check for hdr_offset before casting it? [ ... ]
quoted hunk ↗ jump to hunk
diff --git a/scripts/gen_lineinfo.c b/scripts/gen_lineinfo.c index 699e760178f09..7624098d0cb42 100644 --- a/scripts/gen_lineinfo.c +++ b/scripts/gen_lineinfo.c
[ ... ]
+static void apply_one_dl_reloc(Elf_Data *dl_data, Elf_Data *sym_data,
+ bool target_le,
+ unsigned int abs32_type, unsigned int abs64_type,
+ GElf_Xword r_info, GElf_Addr r_offset,
+ GElf_Sxword addend, bool has_addend)
+{
+ GElf_Sym sym;
+ unsigned int r_type = GELF_R_TYPE(r_info);
+ size_t r_sym = GELF_R_SYM(r_info);
+ bool is_abs64;
+ size_t width;
+ uint64_t value;[ ... ]
+ width = is_abs64 ? 8 : 4; + + if (r_offset + width > dl_data->d_size) + return;
[Severity: Medium] Can this bounds check overflow? Because r_offset is a 64-bit integer, a maliciously crafted .ko file fed to the gen_lineinfo tool could provide an extremely large value (such as 0xFFFFFFFFFFFFFFFE). Adding width to it would wrap around to a small number, successfully bypassing the bounds check. This could lead to out-of-bounds pointer arithmetic and a subsequent out-of-bounds write in elf_write_uint() below, potentially causing a crash or code execution within the kernel build environment. [ ... ]
+static void resolve_covered_sections(Elf *elf,
+ struct covered_section *sections,
+ unsigned int num_sections)
+{[ ... ]
+ if (!covered &&
+ (shdr.sh_flags & SHF_EXECINSTR) && (shdr.sh_flags & SHF_ALLOC) &&
+ shdr.sh_size && shdr.sh_size <= UINT_MAX) {
+ struct covered_section *es;
+
+ extra_sections = realloc(extra_sections,
+ (num_extra_sections + 1) *
+ sizeof(*extra_sections));[ ... ]
quoted hunk ↗ jump to hunk
@@ -552,6 +1387,5 @@ int main(int argc, char *argv[]) for (unsigned int i = 0; i < num_files; i++) free(files[i].name); free(files); - return 0; }
[Severity: Low] Does this code leak the dynamically allocated extra_sections array? In resolve_covered_sections(), extra_sections is allocated and expanded via realloc(). However, the cleanup block in main() only frees entries and files, missing the extra_sections array. While this is a short-lived userspace tool and the OS will automatically reclaim the memory upon exit, would it be better to explicitly free extra_sections to keep the cleanup complete? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260709163833.3851179-1-sashal@kernel.org?part=2