Hi Petr

On 9/9/26 19:30, Petr Pavlu wrote:
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().


I kept this part unmodified to preserve the loop's original intent.

This loop counts SHT_NOTE sections.

SHT_NOTE refers to ELF note sections, which hold non-executable

metadata such as build ID and ABI info.

I wonder if we could keep the current implementation.

As noted in the comment above, the top part counts SHT_NOTE sections

and allocates structures, while the lower logic handles control of node attributes.


WDYT?


Thanks

Best Regards

Hao


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]/


Reply via email to