[PATCH v9 1/4] alloc_tag: move release_module_tags() above reserve_module_tags()

[PATCH v9 1/4] alloc_tag: move release_module_tags() above reserve_module_tags()

[Hao Ge]

release_module_tags() is a cleanup helper. reserve_module_tags() can
also fail after storing the reservation in the maple tree, in which
case it should call release_module_tags() to undo it. Move the helper
above reserve_module_tags() so no forward declaration is needed.

No functional change.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Fixes: 0f9b685626da ("alloc_tag: populate memory for module tags as needed")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Acked-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 mm/alloc_tag.c | 92 +++++++++++++++++++++++++-------------------------
 1 file changed, 46 insertions(+), 46 deletions(-)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index b1d48532a25a..e7a79116ad81 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -843,6 +843,52 @@ static int vm_module_tags_populate(void)
 	return 0;
 }
 
+static void release_module_tags(struct module *mod, bool used)
+{
+	MA_STATE(mas, &mod_area_mt, module_tags.size, module_tags.size);
+	struct alloc_tag *start_tag;
+	struct alloc_tag *end_tag;
+	struct module *val;
+
+	mas_lock(&mas);
+	mas_for_each_rev(&mas, val, 0)
+		if (val == mod)
+			break;
+
+	if (!val) /* module not found */
+		goto out;
+
+	if (!used)
+		goto release_area;
+
+	start_tag = (struct alloc_tag *)(module_tags.start_addr + mas.index);
+	end_tag = (struct alloc_tag *)(module_tags.start_addr + mas.last);
+	if (!clean_unused_counters(start_tag, end_tag)) {
+		struct alloc_tag *tag;
+
+		for (tag = start_tag; tag <= end_tag; tag++) {
+			struct alloc_tag_counters counter;
+
+			if (!tag->counters)
+				continue;
+
+			counter = alloc_tag_read(tag);
+			pr_info("%s:%u module %s func:%s has %llu allocated at module unload\n",
+				tag->ct.filename, tag->ct.lineno, tag->ct.modname,
+				tag->ct.function, counter.bytes);
+		}
+	} else {
+		used = false;
+	}
+release_area:
+	mas_store(&mas, used ? &unloaded_mod : NULL);
+	val = mas_prev_range(&mas, 0);
+	if (val == &prepend_mod)
+		mas_store(&mas, NULL);
+out:
+	mas_unlock(&mas);
+}
+
 static void *reserve_module_tags(struct module *mod, unsigned long size,
 				 unsigned int prepend, unsigned long align)
 {
@@ -930,52 +976,6 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 	return (struct alloc_tag *)(module_tags.start_addr + offset);
 }
 
-static void release_module_tags(struct module *mod, bool used)
-{
-	MA_STATE(mas, &mod_area_mt, module_tags.size, module_tags.size);
-	struct alloc_tag *start_tag;
-	struct alloc_tag *end_tag;
-	struct module *val;
-
-	mas_lock(&mas);
-	mas_for_each_rev(&mas, val, 0)
-		if (val == mod)
-			break;
-
-	if (!val) /* module not found */
-		goto out;
-
-	if (!used)
-		goto release_area;
-
-	start_tag = (struct alloc_tag *)(module_tags.start_addr + mas.index);
-	end_tag = (struct alloc_tag *)(module_tags.start_addr + mas.last);
-	if (!clean_unused_counters(start_tag, end_tag)) {
-		struct alloc_tag *tag;
-
-		for (tag = start_tag; tag <= end_tag; tag++) {
-			struct alloc_tag_counters counter;
-
-			if (!tag->counters)
-				continue;
-
-			counter = alloc_tag_read(tag);
-			pr_info("%s:%u module %s func:%s has %llu allocated at module unload\n",
-				tag->ct.filename, tag->ct.lineno, tag->ct.modname,
-				tag->ct.function, counter.bytes);
-		}
-	} else {
-		used = false;
-	}
-release_area:
-	mas_store(&mas, used ? &unloaded_mod : NULL);
-	val = mas_prev_range(&mas, 0);
-	if (val == &prepend_mod)
-		mas_store(&mas, NULL);
-out:
-	mas_unlock(&mas);
-}
-
 static int load_module(struct module *mod, struct codetag *start, struct codetag *stop)
 {
 	/* Allocate module alloc_tag percpu counters */
-- 
2.25.1

[PATCH v9 2/4] alloc_tag: clean up the populate failure path

[Hao Ge]

The reservation is already stored in the maple tree when
vm_module_tags_populate() fails. A failed load never unloads the
module, so nothing releases the entry. Release it and roll
module_tags.size back. Without the rollback a concurrent load that
already passed needs_section_mem() can reuse the freed gap, skip
vm_module_tags_populate() and write to unmapped memory.

vmap_pages_range() may have installed some PTEs before failing. A
retry to populate the same range would BUG on them, so undo them,
but only if vmap actually ran.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Fixes: 0f9b685626da ("alloc_tag: populate memory for module tags as needed")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 mm/alloc_tag.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index e7a79116ad81..e7a40a276ed9 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -812,6 +812,13 @@ static int vm_module_tags_populate(void)
 				     next_page, PAGE_SHIFT) < 0) {
 			release_pages_arg arg = { .pages = next_page };
 
+			/*
+			 * vmap_pages_range() only runs once all pages were
+			 * allocated, and it may have installed some mappings
+			 * before failing. Undo them.
+			 */
+			if (nr == more_pages)
+				vunmap_range(phys_end, phys_end + (nr << PAGE_SHIFT));
 			/* Clean up and error out */
 			release_pages(arg, nr);
 			return -ENOMEM;
@@ -955,6 +962,7 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 		return ret;
 
 	if (module_tags.size < offset + size) {
+		unsigned long prev_size = module_tags.size;
 		int grow_res;
 
 		module_tags.size = offset + size;
@@ -969,6 +977,8 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 			shutdown_mem_profiling(true);
 			pr_err("Failed to allocate memory for allocation tags in the module %s. Memory allocation profiling is disabled!\n",
 			       mod->name);
+			release_module_tags(mod, false);
+			module_tags.size = prev_size;
 			return ERR_PTR(grow_res);
 		}
 	}
-- 
2.25.1

[PATCH v9 3/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[Hao Ge]

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 <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 include/linux/module.h   |  2 ++
 kernel/module/internal.h |  8 ++++++++
 kernel/module/kallsyms.c | 13 +++----------
 kernel/module/main.c     | 32 +++++++++++++++++---------------
 kernel/module/sysfs.c    | 15 ++++++++++++---
 5 files changed, 42 insertions(+), 28 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 7566815fabbe..33548daa31a3 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -325,6 +325,8 @@ enum mod_mem_type {
 	MOD_INIT_RODATA,
 
 	MOD_MEM_NUM_TYPES,
+
+	MOD_STANDALONE = -2,
 	MOD_INVALID = -1,
 };
 
diff --git a/kernel/module/internal.h b/kernel/module/internal.h
index 061161cc79d9..4c738074a27b 100644
--- a/kernel/module/internal.h
+++ b/kernel/module/internal.h
@@ -29,6 +29,14 @@
 #define SH_ENTSIZE_TYPE_MASK	((1UL << SH_ENTSIZE_TYPE_BITS) - 1)
 #define SH_ENTSIZE_OFFSET_MASK	((1UL << (BITS_PER_LONG - SH_ENTSIZE_TYPE_BITS)) - 1)
 
+/*
+ * Marker for sections with a separate allocation, which are not placed
+ * into mod->mem[].
+ */
+#define SH_ENTSIZE_STANDALONE					\
+	(((unsigned long)MOD_STANDALONE & SH_ENTSIZE_TYPE_MASK)	\
+	 << SH_ENTSIZE_TYPE_SHIFT)
+
 /* Maximum number of characters written by module_flags() */
 #define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
 
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] =
diff --git a/kernel/module/main.c b/kernel/module/main.c
index c32f1d370b73..fc577c01dfd2 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1620,12 +1620,13 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
 
 		/*
 		 * Don't bother with non-allocated sections.
-		 * An exception is the percpu section, which has separate allocations
-		 * for individual CPUs. We relocate the percpu section in the initial
-		 * ELF template and subsequently copy it to the per-CPU destinations.
+		 *
+		 * Note that .data..percpu has separate allocations for
+		 * individual CPUs. We relocate the section in the
+		 * initial ELF template and subsequently copy it to the
+		 * per-CPU destinations.
 		 */
-		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC) &&
-		    (!infosec || infosec != info->index.pcpu))
+		if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC))
 			continue;
 
 		if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
@@ -1716,7 +1717,7 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
 
 			if ((s->sh_flags & masks[m][0]) != masks[m][0]
 			    || (s->sh_flags & masks[m][1])
-			    || s->sh_entsize != ~0UL
+			    || s->sh_entsize != ~0UL /* offset or standalone */
 			    || is_init != module_init_layout_section(sname))
 				continue;
 
@@ -1746,16 +1747,10 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
 /*
  * Lay out the SHF_ALLOC sections in a way not dissimilar to how ld
  * might -- code, read-only data, read-write data, small data.  Tally
- * sizes, and place the offsets into sh_entsize fields: high bit means it
- * belongs in init.
+ * sizes, and place the offsets into sh_entsize fields.
  */
 static void layout_sections(struct module *mod, struct load_info *info)
 {
-	unsigned int i;
-
-	for (i = 0; i < info->hdr->e_shnum; i++)
-		info->sechdrs[i].sh_entsize = ~0UL;
-
 	pr_debug("Core section allocation order for %s:\n", mod->name);
 	__layout_sections(mod, info, false);
 
@@ -2811,7 +2806,8 @@ static int move_module(struct module *mod, struct load_info *info)
 		Elf_Shdr *shdr = &info->sechdrs[i];
 		const char *sname;
 
-		if (!(shdr->sh_flags & SHF_ALLOC))
+		if (!(shdr->sh_flags & SHF_ALLOC)
+		    || shdr->sh_entsize == SH_ENTSIZE_STANDALONE)
 			continue;
 
 		sname = info->secstrings + shdr->sh_name;
@@ -2943,6 +2939,7 @@ core_param(module_blacklist, module_blacklist, charp, 0400);
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	struct module *mod;
+	unsigned int i;
 	int err;
 
 	/* Allow arches to frob section contents and sizes.  */
@@ -2956,8 +2953,13 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	if (err < 0)
 		return ERR_PTR(err);
 
+	/* Repurpose sh_entsize to track where each section is allocated. */
+	for (i = 0; i < info->hdr->e_shnum; i++)
+		info->sechdrs[i].sh_entsize = ~0UL;
+
 	/* We will do a special allocation for per-cpu sections later. */
-	info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
+	if (info->index.pcpu)
+		info->sechdrs[info->index.pcpu].sh_entsize = SH_ENTSIZE_STANDALONE;
 
 	/*
 	 * Mark relevant sections as SHF_RO_AFTER_INIT so layout_sections() can
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);
-- 
2.25.1

[PATCH v9 4/4] module: allocate codetag sections before the regular module layout

[Hao Ge]

Whether a codetag section goes to the codetag region is decided by
layout_sections() and asked again in move_module(). A concurrent
load can shut profiling down in between, and move_module() then
copies the section to offset 0 of its regular destination,
overwriting whatever is there.

Decide and allocate in one pass, before the layout. Allocation
errors fail the load. On a tag area overflow profiling is already
disabled, so -EAGAIN makes the section fall back to regular module
data and the module still loads. The reservation is released and
module_tags.size rolled back, so a concurrent load which already
passed needs_section_mem() does not skip vm_module_tags_populate()

An SHT_NOBITS codetag section is zeroed explicitly, the tag area
pages are not zeroed on allocation.

When profiling was toggled off the overflow check did not run, a
module could load with more tags than the page flags can address,
and re-enabling profiling then silently corrupted /proc/allocinfo.
The check no longer depends on mem_alloc_profiling_enabled().

Based on a patch by Petr Pavlu [1].

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 kernel/module/main.c | 101 +++++++++++++++++++++++--------------------
 mm/alloc_tag.c       |   9 ++--
 2 files changed, 60 insertions(+), 50 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index fc577c01dfd2..af8880d430fb 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1724,20 +1724,6 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
 			if (WARN_ON_ONCE(type == MOD_INVALID))
 				continue;
 
-			/*
-			 * Do not allocate codetag memory as we load it into
-			 * preallocated contiguous memory.
-			 */
-			if (codetag_needs_module_section(mod, sname, s->sh_size)) {
-				/*
-				 * s->sh_entsize won't be used but populate the
-				 * type field to avoid confusion.
-				 */
-				s->sh_entsize = ((unsigned long)(type) & SH_ENTSIZE_TYPE_MASK)
-						<< SH_ENTSIZE_TYPE_SHIFT;
-				continue;
-			}
-
 			s->sh_entsize = module_get_offset_and_type(mod, type, s, i);
 			pr_debug("\t%s\n", sname);
 		}
@@ -2784,7 +2770,6 @@ static int move_module(struct module *mod, struct load_info *info)
 {
 	int i, ret;
 	enum mod_mem_type t = MOD_MEM_NUM_TYPES;
-	bool codetag_section_found = false;
 
 	for_each_mod_mem_type(type) {
 		if (!mod->mem[type].size) {
@@ -2804,35 +2789,13 @@ static int move_module(struct module *mod, struct load_info *info)
 	for (i = 0; i < info->hdr->e_shnum; i++) {
 		void *dest;
 		Elf_Shdr *shdr = &info->sechdrs[i];
-		const char *sname;
 
 		if (!(shdr->sh_flags & SHF_ALLOC)
 		    || shdr->sh_entsize == SH_ENTSIZE_STANDALONE)
 			continue;
 
-		sname = info->secstrings + shdr->sh_name;
-		/*
-		 * Load codetag sections separately as they might still be used
-		 * after module unload.
-		 */
-		if (codetag_needs_module_section(mod, sname, shdr->sh_size)) {
-			dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
-					arch_mod_section_prepend(mod, i), shdr->sh_addralign);
-			if (WARN_ON(!dest)) {
-				ret = -EINVAL;
-				goto out_err;
-			}
-			if (IS_ERR(dest)) {
-				ret = PTR_ERR(dest);
-				goto out_err;
-			}
-			codetag_section_found = true;
-		} else {
-			enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT;
-			unsigned long offset = shdr->sh_entsize & SH_ENTSIZE_OFFSET_MASK;
-
-			dest = mod->mem[type].base + offset;
-		}
+		dest = mod->mem[shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT].base +
+		       (shdr->sh_entsize & SH_ENTSIZE_OFFSET_MASK);
 
 		if (shdr->sh_type != SHT_NOBITS) {
 			/*
@@ -2864,8 +2827,6 @@ static int move_module(struct module *mod, struct load_info *info)
 	module_memory_restore_rox(mod);
 	while (t--)
 		module_memory_free(mod, t);
-	if (codetag_section_found)
-		codetag_free_module_sections(mod);
 
 	return ret;
 }
@@ -2936,6 +2897,49 @@ static bool blacklisted(const char *module_name)
 }
 core_param(module_blacklist, module_blacklist, charp, 0400);
 
+/*
+ * Allocate codetag sections separately. They are loaded into preallocated
+ * contiguous memory because they may still be used after the module is
+ * unloaded.
+ *
+ * If the separate allocation overflows, allocate the section normally
+ * so that the module can still be loaded.
+ */
+static int allocate_codetag_sections(struct load_info *info)
+{
+	for (unsigned int i = 1; i < info->hdr->e_shnum; i++) {
+		Elf_Shdr *shdr = &info->sechdrs[i];
+		const char *sname = info->secstrings + shdr->sh_name;
+		void *dest;
+
+		if (!codetag_needs_module_section(info->mod, sname, shdr->sh_size))
+			continue;
+
+		dest = codetag_alloc_module_section(info->mod, sname, shdr->sh_size,
+				arch_mod_section_prepend(info->mod, i), shdr->sh_addralign);
+		if (WARN_ON(!dest)) {
+			codetag_free_module_sections(info->mod);
+			return -EINVAL;
+		}
+		if (dest == ERR_PTR(-EAGAIN))
+			/* Allocate the section as a regular section. */
+			continue;
+		if (IS_ERR(dest)) {
+			codetag_free_module_sections(info->mod);
+			return PTR_ERR(dest);
+		}
+
+		if (shdr->sh_type != SHT_NOBITS)
+			memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
+		else
+			memset(dest, 0, shdr->sh_size);
+		shdr->sh_addr = (unsigned long)dest;
+		shdr->sh_entsize = SH_ENTSIZE_STANDALONE;
+	}
+
+	return 0;
+}
+
 static struct module *layout_and_allocate(struct load_info *info, int flags)
 {
 	struct module *mod;
@@ -2968,18 +2972,21 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	 */
 	module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
 
-	/*
-	 * Determine total sizes, and put offsets in sh_entsize.  For now
-	 * this is done generically; there doesn't appear to be any
-	 * special cases for the architectures.
-	 */
+	/* Allow codetag sections to be allocated separately first. */
+	err = allocate_codetag_sections(info);
+	if (err)
+		return ERR_PTR(err);
+
+	/* Determine total sizes and put offsets in sh_entsize. */
 	layout_sections(info->mod, info);
 	layout_symtab(info->mod, info);
 
 	/* Allocate and move to the final place */
 	err = move_module(info->mod, info);
-	if (err)
+	if (err) {
+		codetag_free_module_sections(info->mod);
 		return ERR_PTR(err);
+	}
 
 	/* Module has been copied to its final place now: return it. */
 	mod = (void *)info->sechdrs[info->index.mod].sh_addr;
diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index e7a40a276ed9..76942f2e3905 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -966,10 +966,13 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 		int grow_res;
 
 		module_tags.size = offset + size;
-		if (mem_alloc_profiling_enabled() && !tags_addressable()) {
+		if (!tags_addressable()) {
 			shutdown_mem_profiling(true);
-			pr_warn("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
-				mod->name, NR_UNUSED_PAGEFLAG_BITS);
+			pr_warn_once("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
+				     mod->name, NR_UNUSED_PAGEFLAG_BITS);
+			release_module_tags(mod, false);
+			module_tags.size = prev_size;
+			return ERR_PTR(-EAGAIN);
 		}
 
 		grow_res = vm_module_tags_populate();
-- 
2.25.1

Re: [PATCH v9 3/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[Petr Pavlu]

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 <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> ---
> [...]
> 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 <petr.pavlu@suse.com>

[1] https://lore.kernel.org/linux-modules/20260820125007.22943-1-yangtiezhu@loongson.cn/

-- 
Thanks,
Petr

Re: [PATCH v9 4/4] module: allocate codetag sections before the regular module layout

[Petr Pavlu]

On 9/8/26 11:24 AM, Hao Ge wrote:
> Whether a codetag section goes to the codetag region is decided by
> layout_sections() and asked again in move_module(). A concurrent
> load can shut profiling down in between, and move_module() then
> copies the section to offset 0 of its regular destination,
> overwriting whatever is there.
> 
> Decide and allocate in one pass, before the layout. Allocation
> errors fail the load. On a tag area overflow profiling is already
> disabled, so -EAGAIN makes the section fall back to regular module
> data and the module still loads. The reservation is released and
> module_tags.size rolled back, so a concurrent load which already
> passed needs_section_mem() does not skip vm_module_tags_populate()
> 
> An SHT_NOBITS codetag section is zeroed explicitly, the tag area
> pages are not zeroed on allocation.
> 
> When profiling was toggled off the overflow check did not run, a
> module could load with more tags than the page flags can address,
> and re-enabling profiling then silently corrupted /proc/allocinfo.
> The check no longer depends on mem_alloc_profiling_enabled().
> 
> Based on a patch by Petr Pavlu [1].
> 
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
> Cc: stable@vger.kernel.org
> Signed-off-by: Hao Ge <hao.ge@linux.dev>

This looks ok to me from the module loader's perspective.

Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>

-- 
Thanks,
Petr

Re: [PATCH v9 3/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[Hao Ge]

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 <sashiko-bot@kernel.org>
>> Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
>> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>> [...]
>> 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 <petr.pavlu@suse.com>
>
> [1] https://lore.kernel.org/linux-modules/20260820125007.22943-1-yangtiezhu@loongson.cn/
>

Re: [PATCH v9 3/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[Hao Ge]

On 9/9/26 20:47, Hao Ge wrote:
> 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 <sashiko-bot@kernel.org>
>>> Link: 
>>> https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ 
>>> [1]
>>> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
>>> Cc: stable@vger.kernel.org
>>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>>> ---
>>> [...]
>>> 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?
>
>

Sorry, I've reconsidered this. I think changing it to sect_visible would 
be better.

sect_visible stands for the count of externally visible note attributes, 
so the code

above and below can align with each other.


Sorry for the noise.


I'll rebase onto modules-next, apply this change and push a new revision.

I'll also add your Reviewed-by tag.


Thanks

Best Regards

Hao


> Thanks
>
> Best Regards
>
> Hao
>
>>
>> With this fixed, the patch looks ok to me. Feel free to add:
>>
>> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com>
>>
>> [1] 
>> https://lore.kernel.org/linux-modules/20260820125007.22943-1-yangtiezhu@loongson.cn/
>>

Re: [PATCH v9 3/4] module: introduce SH_ENTSIZE_STANDALONE for separately allocated sections

[Petr Pavlu]

On 9/9/26 3:08 PM, Hao Ge wrote:
> On 9/9/26 20:47, Hao Ge wrote:
>> 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 <sashiko-bot@kernel.org>
>>>> Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/ [1]
>>>> Suggested-by: Petr Pavlu <petr.pavlu@suse.com>
>>>> Cc: stable@vger.kernel.org
>>>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>>>> ---
>>>> [...]
>>>> 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?
>>
>>
> 
> Sorry, I've reconsidered this. I think changing it to sect_visible would be better.
> 
> sect_visible stands for the count of externally visible note attributes, so the code
> 
> above and below can align with each other.
> 
> 
> Sorry for the noise.

No worries.

One problem with continuing to use sect_empty() in the first loop is
that it could overallocate the number of required entries if a module
contains an SHT_NOTE section named .data..percpu. That shouldn't happen,
but we can trivially avoid it by using sect_visible() there as well.

Using the same condition in both loops also makes the code generally
simpler to understand.

-- 
Cheers,
Petr