Re: [PATCH v6 2/4] kallsyms: extend lineinfo to loadable modules

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] `apply_debug_line_relocations()` ignores `.rel.debug_line` (SHT_REL), breaking module lineinfo on 32-bit architectures like i386 and ARM.
- [High] Misattribution of DWARF line entries for unhandled text sections pollutes `.text` lineinfo.
- [High] Hardcoded assembly padding creates an ABI mismatch with the C struct layout on architectures where `alignof(u64) == 4` (e.g., 32-bit x86).
- [Medium] An integer overflow in relocation offset validation allows an out-of-bounds write in the `gen_lineinfo` build tool.
--

commit a6a5ac22ccec63c8ea45f4ded1db6418cd100063
Author: Sasha Levin <sashal@kernel.org>
Subject: kallsyms: extend lineinfo to loadable modules

This commit extends the CONFIG_KALLSYMS_LINEINFO feature to loadable kernel
modules. It post-processes modules at build time to generate per-section
.mod_lineinfo binary tables mapping addresses to source lines. The tables
are embedded in the module and searched during stack traces.

> diff --git a/scripts/gen_lineinfo.c b/scripts/gen_lineinfo.c
> index 699e760178f09..e1e08469b4f2f 100644
> --- a/scripts/gen_lineinfo.c
> +++ b/scripts/gen_lineinfo.c
[ ... ]
> +	/* Find the relevant sections */
> +	while ((scn = elf_nextscn(elf, scn)) != NULL) {
> +		const char *name;
> +
> +		if (!gelf_getshdr(scn, &shdr))
> +			continue;
> +		name = elf_strptr(elf, shstrndx, shdr.sh_name);
> +		if (!name)
> +			continue;
> +
> +		if (!strcmp(name, ".debug_line"))
> +			debug_line_scn = scn;
> +		else if (!strcmp(name, ".rela.debug_line"))
> +			rela_debug_line_scn = scn;
> +		else if (shdr.sh_type == SHT_SYMTAB)
> +			symtab_scn = scn;
> +	}
> +
> +	if (!debug_line_scn || !rela_debug_line_scn || !symtab_scn)
> +		return;

[Severity: High]
Does apply_debug_line_relocations() skip .rel.debug_line sections?

On 32-bit architectures like i386 and ARM, relocations use SHT_REL and
generate a .rel.debug_line section rather than .rela.debug_line. 

If the function bails out here, synthetic biases are never added and
DW_FORM_line_strp offsets remain un-relocated, which feeds zeros to libdw.
Could this break the module lineinfo feature entirely on these architectures?

[ ... ]
> +		if (module_mode) {
> +			struct covered_section *cs;
> +
> +			cs = section_by_index(all_sections, ALL_SECTIONS,
> +					      sym.st_shndx);
> +			if (cs)
> +				value += cs->bias;
> +		}

[Severity: High]
Could unbiased text sections pollute the .text lineinfo?

Sections not listed in all_sections[], such as .noinstr.text, do not
receive a synthetic bias here, leaving their relocated DWARF PCs starting at 0.

When classify_address() processes these entries:

classify_address() {
    ...
    if (addr < sections[i].bias)
        continue;
    if (addr >= sections[i].bias + sections[i].size)
        continue;
    ...
}

An address smaller than the size of .text will fall into the .text bucket,
since .text has a bias of 0. Would this incorrectly merge line entries and
corrupt the binary search invariant?

[ ... ]
> +		if (!is_abs64)
> +			value &= 0xffffffffULL;
> +
> +		if (rela.r_offset + width <= dl_data->d_size)
> +			elf_write_uint((unsigned char *)dl_data->d_buf +
> +					       rela.r_offset,
> +				       value, width, target_le);
> +	}

[Severity: Medium]
Is there a risk of an integer overflow in this bounds check?

If a maliciously crafted ELF file provides a huge r_offset value from the
relocation section (such as UINT64_MAX), the addition of width could wrap
around to a small positive integer and bypass the check. This might lead to
an out-of-bounds write in elf_write_uint().

[ ... ]
> +static void emit_blob(const char *output_section,
> +		      const char *blob_tag,
> +		      enum mod_lineinfo_blob blob)
> +{
[ ... ]
> +	printf("\t.section %s, \"a\"\n\n", output_section);
> +
> +	printf("\t.balign 8\n");
> +	printf(".Lroot_%s:\n", blob_tag);
> +	printf("\t.long %u\t\t/* num_sections */\n", active);
> +	/* Pad to align the u64 anchor in sections[0] to 8 bytes. */
> +	printf("\t.balign 8\n");

[Severity: High]
Does this hardcoded padding create an ABI mismatch with the mod_lineinfo_root
C struct on some architectures?

The assembly forces an 8-byte alignment immediately following the 4-byte
num_sections field. However, on 32-bit x86 architectures, the System V ABI
sets alignof(u64) to 4.

Looking at include/linux/mod_lineinfo.h:

struct mod_lineinfo_root {
    u32 num_sections;
    struct mod_lineinfo_section sections[];
};

The C compiler will align the struct to 4 bytes, meaning the sections[]
flexible array starts at offset 4, not offset 8. If the kernel reads this
blob, could it parse the assembly padding as the start of the anchor field
and corrupt the section descriptors?

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