On 9/8/26 11:24 AM, Hao Ge wrote:
> SHF_ALLOC means, per the ELF spec, that a section occupies memory
> during process execution. Some module sections occupy memory
> outside the regular module layout, for example the percpu section
> with its per-CPU allocations. The loader currently excludes such
> a section from the layout by clearing its SHF_ALLOC, which
> overloads the flag with a loader-internal meaning.
> apply_relocations() needs a special case for the section, and
> find_sec(".data..percpu") returns different results before and
> after layout_and_allocate().
> 
> Introduce SH_ENTSIZE_STANDALONE to mark sections with a separate
> allocation. The percpu section is its first user. layout_sections()
> and move_module() skip marked sections, and apply_relocations() goes
> back to testing only SHF_ALLOC. Based on a patch by Petr Pavlu [1].
> 
> .data..percpu keeps SHF_ALLOC, so it would now show up under
> /sys/module/*/sections/. The section has one instance per CPU and no
> single address to report, and the entry never existed before, so
> skip it in add_sect_attrs(). add_notes_attrs() indexes its attrs[]
> array and skips it too. No functional change otherwise.
> 
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Reported-by: Sashiko <[email protected]>
> Link: 
> https://lore.kernel.org/all/[email protected]/ [1]
> Suggested-by: Petr Pavlu <[email protected]>
> Cc: [email protected]
> Signed-off-by: Hao Ge <[email protected]>
> ---
> [...]
> diff --git a/kernel/module/kallsyms.c b/kernel/module/kallsyms.c
> index 0fc11e45df9b..49190deae61e 100644
> --- a/kernel/module/kallsyms.c
> +++ b/kernel/module/kallsyms.c
> @@ -76,7 +76,7 @@ static char elf_type(const Elf_Sym *sym, const struct 
> load_info *info)
>  }
>  
>  static bool is_core_symbol(const Elf_Sym *src, const Elf_Shdr *sechdrs,
> -                        unsigned int shnum, unsigned int pcpundx)
> +                        unsigned int shnum)
>  {
>       const Elf_Shdr *sec;
>       enum mod_mem_type type;
> @@ -86,11 +86,6 @@ static bool is_core_symbol(const Elf_Sym *src, const 
> Elf_Shdr *sechdrs,
>           !src->st_name)
>               return false;
>  
> -#ifdef CONFIG_KALLSYMS_ALL
> -     if (src->st_shndx == pcpundx)
> -             return true;
> -#endif
> -
>       sec = sechdrs + src->st_shndx;
>       type = sec->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT;
>       if (!(sec->sh_flags & SHF_ALLOC)
> @@ -131,8 +126,7 @@ 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 (i == 0 || is_livepatch_module(mod) ||
> -                 is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
> -                                info->index.pcpu)) {
> +                 is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum)) 
> {
>                       strtab_size += strlen(&info->strtab[src[i].st_name]) + 
> 1;
>                       ndst++;
>               }
> @@ -199,8 +193,7 @@ void add_kallsyms(struct module *mod, const struct 
> load_info *info)
>       for (ndst = i = 0; i < kallsyms->num_symtab; i++) {
>               kallsyms->typetab[i] = elf_type(src + i, info);
>               if (i == 0 || is_livepatch_module(mod) ||
> -                 is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum,
> -                                info->index.pcpu)) {
> +                 is_core_symbol(src + i, info->sechdrs, info->hdr->e_shnum)) 
> {
>                       ssize_t ret;
>  
>                       mod->core_kallsyms.typetab[ndst] =

FTR These changes in kernel/module/kallsyms.c have a conflict with the
series "Ignore local labels and mapping symbols during module load" [1],
which is currently queued on modules-next, but it should be
straightforward to resolve.

> diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c
> index 01c65d608873..f64170344e69 100644
> --- a/kernel/module/sysfs.c
> +++ b/kernel/module/sysfs.c
> @@ -62,6 +62,15 @@ static void free_sect_attrs(struct module_sect_attrs 
> *sect_attrs)
>       kfree(sect_attrs);
>  }
>  
> +/*
> + * .data..percpu has a separate allocation per CPU and no single
> + * address to report.
> + */
> +static bool sect_visible(const struct load_info *info, unsigned int i)
> +{
> +     return !sect_empty(&info->sechdrs[i]) && i != info->index.pcpu;
> +}
> +
>  static int add_sect_attrs(struct module *mod, const struct load_info *info)
>  {
>       struct module_sect_attrs *sect_attrs;
> @@ -72,7 +81,7 @@ static int add_sect_attrs(struct module *mod, const struct 
> load_info *info)
>  
>       /* Count loaded sections and allocate structures */
>       for (i = 0; i < info->hdr->e_shnum; i++)
> -             if (!sect_empty(&info->sechdrs[i]))
> +             if (sect_visible(info, i))
>                       nloaded++;
>       sect_attrs = kzalloc_flex(*sect_attrs, attrs, nloaded);
>       if (!sect_attrs)
> @@ -92,7 +101,7 @@ static int add_sect_attrs(struct module *mod, const struct 
> load_info *info)
>       for (i = 0; i < info->hdr->e_shnum; i++) {
>               Elf_Shdr *sec = &info->sechdrs[i];
>  
> -             if (sect_empty(sec))
> +             if (!sect_visible(info, i))
>                       continue;
>               sysfs_bin_attr_init(sattr);
>               sattr->attr.name =
> @@ -181,7 +190,7 @@ static int add_notes_attrs(struct module *mod, const 
> struct load_info *info)
>  
>       nattr = &notes_attrs->attrs[0];
>       for (loaded = i = 0; i < info->hdr->e_shnum; ++i) {
> -             if (sect_empty(&info->sechdrs[i]))
> +             if (!sect_visible(info, i))
>                       continue;
>               if (info->sechdrs[i].sh_type == SHT_NOTE) {
>                       sysfs_bin_attr_init(nattr);

add_notes_attrs() has two sect_empty() calls. Both should be changed to
sect_visible().

With this fixed, the patch looks ok to me. Feel free to add:

Reviewed-by: Petr Pavlu <[email protected]>

[1] 
https://lore.kernel.org/linux-modules/[email protected]/

-- 
Thanks,
Petr

Reply via email to