Re: [PATCH v1] module/kallsyms: Filter out local mapping symbols during module load

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Filtering mapping symbols from `core_kallsyms.symtab` before checking for livepatch modules fundamentally breaks the 1:1 symbol index mapping required for livepatch delayed relocations.
--

--- Patch [1]: [PATCH v1] module/kallsyms: Filter out local mapping symbols during module load ---
commit 341d5ac5814c0e09a7b6cda2b0753c3161fb7f96
Author: Tiezhu Yang <[email protected]>

module/kallsyms: Filter out local mapping symbols during module load

This patch filters out compiler-generated local mapping symbols from loaded
modules' permanent symbol tables. It modifies layout_symtab() and
add_kallsyms() to skip these mapping symbols during layout generation so
that tracing tools resolve to clear C function names instead.

> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -130,6 +130,9 @@ void layout_symtab(struct module *mod, struct load_info *info)
>  
>  	/* Compute total space required for the core symbols' strtab. */
>  	for (ndst = i = 0; i < nsrc; i++) {
> +		if (is_mapping_symbol(&info->strtab[src[i].st_name]))
> +			continue;
> +
>  		if (i == 0 || is_livepatch_module(mod) ||
>  		    is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
>  				   info->index.pcpu)) {

[Severity: High]
Does skipping mapping symbols before the is_livepatch_module() check
incorrectly alter the total symbol count and layout for livepatch modules?

The livepatch subsystem requires mod->core_kallsyms.symtab to be an exact,
1:1 copy of the original ELF symbol table.

[ ... ]

> @@ -198,6 +201,10 @@ void add_kallsyms(struct module *mod, const struct load_info *info)
>  	src = kallsyms->symtab;
>  	for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
>  		kallsyms->typetab[i] = elf_type(src + i, info);
> +
> +		if (is_mapping_symbol(&kallsyms->strtab[src[i].st_name]))
> +			continue;
> +
>  		if (i == 0 || is_livepatch_module(mod) ||
>  		    is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
>  				   info->index.pcpu)) {

[Severity: High]
Will filtering mapping symbols while copying to core_kallsyms.symtab cause
all subsequent symbol indices to physically shift down, breaking livepatch
delayed relocations?

kernel/module/livepatch.c:copy_module_elf() explicitly documents this
requirement:

    /*
     * For livepatch modules, core_kallsyms.symtab is a complete
     * copy of the original symbol table. Adjust sh_addr to point
     * to core_kallsyms.symtab since the copy of the symtab in module
     * init memory is freed at the end of do_init_module().
     */
    mod->klp_info->sechdrs[symndx].sh_addr =
        (unsigned long)mod->core_kallsyms.symtab;

When klp_resolve_symbols() indexes into the table using the original
(unshifted) ELF indices, will it read the wrong symbol or go out of bounds?

kernel/livepatch/core.c:klp_resolve_symbols() {
    ...
        sym = (Elf_Sym *)sechdrs[symndx].sh_addr + ELF_R_SYM(relas[i].r_info);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1