[RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling()

[RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling()

[Hao Ge]

Three fixes for races and a leak in how allocation profiling meets the
module loader, reported by sashiko.

  1. percpu counter leak on modules loaded after profiling is disabled.
  2. layout/move TOCTOU that can silently overwrite module memory when
     profiling disabled between layout_sections() and move_module().
     Additionally, the retry logic here depends on [1].
  3. AB-BA deadlock between module load and /proc/allocinfo readers.

Patch 1 adds CODETAG_MODULE_EXCLUDED so profiling-disabled modules are
dropped from the tag list instead of half-registered, and frees the
section reservation unconditionally on unload. Patch 2 makes
layout_sections() the sole authority for codetag placement. Patch 3
defers remove_proc_entry() to a workqueue.

Feedback and suggestions are warmly welcomed; I would greatly appreciate
any input.

Remaining sashiko-reported bugs will be fixed in follow-up patches.

[1]: https://lore.kernel.org/all/20260812054105.102637-3-hao.ge@linux.dev/

Hao Ge (3):
  alloc_tag: skip percpu counter allocation when profiling is disabled
  module: move codetag section placement decision to layout_sections()
  alloc_tag: remove /proc/allocinfo outside of mod_lock

 include/linux/codetag.h |  4 ++++
 include/linux/module.h  | 11 +++++++++++
 kernel/module/main.c    | 17 ++++++-----------
 lib/codetag.c           |  8 +++++---
 mm/alloc_tag.c          | 25 ++++++++++++++++++++++---
 5 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.25.1

[RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled

[Hao Ge]

After shutdown_mem_profiling() clears mem_profiling_support,
needs_section_mem() returns false, so later modules have their codetag
section placed as regular data and never enter the alloc_tag maple tree.
codetag_load_module() still called load_module(), which allocated a percpu
counter for every tag; release_module_tags() could not find these modules
on unload, so the counters leaked.

Return CODETAG_MODULE_EXCLUDED from load_module() when profiling is off:
codetag_module_init() drops the module's cmod and no counters are
allocated. codetag_unload_module() now always calls free_section_mem(),
since an excluded module may still hold a reserved section.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 include/linux/codetag.h | 4 ++++
 lib/codetag.c           | 8 +++++---
 mm/alloc_tag.c          | 8 ++++++--
 3 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/include/linux/codetag.h b/include/linux/codetag.h
index a25a085c2df1..88081c618673 100644
--- a/include/linux/codetag.h
+++ b/include/linux/codetag.h
@@ -52,6 +52,10 @@ struct codetag_type_desc {
 #endif
 };
 
+/* module_load() return values */
+#define CODETAG_MODULE_LOAD	0	/* module loads with its tags */
+#define CODETAG_MODULE_EXCLUDED	1	/* module loads without its tags */
+
 struct codetag_iterator {
 	struct codetag_type *cttype;
 	struct codetag_module *cmod;
diff --git a/lib/codetag.c b/lib/codetag.c
index a9cda4c962a3..8506ecab9ea7 100644
--- a/lib/codetag.c
+++ b/lib/codetag.c
@@ -238,9 +238,10 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod)
 	}
 	up_write(&cttype->mod_lock);
 
-	if (err < 0) {
+	if (err) {
+		/* Error or excluded: cmod is dropped, free it. */
 		kfree(cmod);
-		return err;
+		return err < 0 ? err : 0;
 	}
 
 	return 0;
@@ -388,7 +389,8 @@ void codetag_unload_module(struct module *mod)
 			++cttype->content_id;
 		}
 		up_write(&cttype->mod_lock);
-		if (found && cttype->desc.free_section_mem)
+		/* an excluded module may still hold section memory */
+		if (cttype->desc.free_section_mem)
 			cttype->desc.free_section_mem(mod, true);
 	}
 	mutex_unlock(&codetag_lock);
diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 0a7b657fe2de..461fa87fbb0b 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -977,9 +977,13 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
 	struct alloc_tag *stop_tag;
 	struct alloc_tag *tag;
 
+	/* Profiling disabled: load the module but exclude its tags. */
+	if (!mem_profiling_support)
+		return CODETAG_MODULE_EXCLUDED;
+
 	/* percpu counters for core allocations are already statically allocated */
 	if (!mod)
-		return 0;
+		return CODETAG_MODULE_LOAD;
 
 	start_tag = ct_to_alloc_tag(start);
 	stop_tag = ct_to_alloc_tag(stop);
@@ -1002,7 +1006,7 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
 		 */
 		kmemleak_ignore_percpu(tag->counters);
 	}
-	return 0;
+	return CODETAG_MODULE_LOAD;
 }
 
 static void replace_module(struct module *mod, struct module *new_mod)
-- 
2.25.1

[RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()

[Hao Ge]

codetag_needs_module_section() is called twice per codetag section, once
in layout_sections() and once in move_module(), and both depend on
mem_profiling_support, which changes without a lock. If profiling is
disabled between the two calls, layout excludes the section (offset 0)
while move copies it as normal memory to offset 0:

  CPU0 (insmod A)                    CPU1 (insmod B)
  ----------------                   ----------------
  layout_sections()
    needs_section_mem() == true
    sh_entsize: type, offset = 0
                                     reserve_module_tags() overflows
                                       shutdown_mem_profiling()
                                       mem_profiling_support = false
  move_module()
    needs_section_mem() == false
    offset = sh_entsize & MASK = 0
    memcpy(mod->mem[type].base + 0, ...)
       -> overwrites the first section there

Record the decision in layout_sections() in sh_entsize using a
MOD_MEM_CODETAG type, and have move_module() use that instead of asking
again.

reserve_module_tags() returns -EAGAIN if profiling was disabled after
layout, so the loader retries and places the section as normal memory.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 include/linux/module.h | 11 +++++++++++
 kernel/module/main.c   | 17 ++++++-----------
 mm/alloc_tag.c         |  8 ++++++++
 3 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index 7566815fabbe..a02016528e1d 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -328,6 +328,17 @@ enum mod_mem_type {
 	MOD_INVALID = -1,
 };
 
+/*
+ * If CONFIG_CODE_TAGGING is on, modules get a .codetag section.
+ * codetag_needs_module_section() says where it goes: the usual
+ * mod->mem[], or off to the codetag region.
+ *
+ * Mark the codetag-region ones with MOD_MEM_NUM_TYPES.
+ * It's just past the real types, so it doesn't index into mod->mem[]
+ * and for_each_mod_mem_type() skips it.
+ */
+#define MOD_MEM_CODETAG	MOD_MEM_NUM_TYPES
+
 #define mod_mem_type_is_init(type)	\
 	((type) == MOD_INIT_TEXT ||	\
 	 (type) == MOD_INIT_DATA ||	\
diff --git a/kernel/module/main.c b/kernel/module/main.c
index ed26f167be84..2337bf604f58 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1728,11 +1728,8 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
 			 * 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)
+				s->sh_entsize = ((unsigned long)MOD_MEM_CODETAG
+						 & SH_ENTSIZE_TYPE_MASK)
 						<< SH_ENTSIZE_TYPE_SHIFT;
 				continue;
 			}
@@ -2815,11 +2812,10 @@ static int move_module(struct module *mod, struct load_info *info)
 			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)) {
+
+		enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT;
+
+		if (type == MOD_MEM_CODETAG) {
 			dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
 					arch_mod_section_prepend(mod, i), shdr->sh_addralign);
 			if (WARN_ON(!dest)) {
@@ -2832,7 +2828,6 @@ static int move_module(struct module *mod, struct load_info *info)
 			}
 			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;
diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 461fa87fbb0b..7481180dadd2 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -893,6 +893,14 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 	if (size < sizeof(struct alloc_tag))
 		return ERR_PTR(-EINVAL);
 
+	/*
+	 * Profiling may have been disabled by a concurrent module load.
+	 * Return -EAGAIN so the loader retries with profiling off, laying
+	 * the section out as ordinary module memory.
+	 */
+	if (!mem_profiling_support)
+		return ERR_PTR(-EAGAIN);
+
 	/*
 	 * align is always power of 2, so we can use IS_ALIGNED and ALIGN.
 	 * align 0 or 1 means no alignment, to simplify set to 1.
-- 
2.25.1

[RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock

[Hao Ge]

shutdown_mem_profiling() calls remove_proc_entry() from
reserve_module_tags(), which runs under mod_lock held for write.
remove_proc_entry() waits for readers, and a reader takes mod_lock for
read in allocinfo_start():

  CPU0 (insmod)                      CPU1 (read /proc/allocinfo)
  ----------------                   ----------------------------
  reserve_module_tags()
    down_write(&mod_lock)  [held]
                                     use_pde()            [in_use++]
                                     allocinfo_start()
                                       down_read(&mod_lock)  <- blocks
    shutdown_mem_profiling()
      remove_proc_entry()
        wait for in_use == 0         <- blocks

Move remove_proc_entry() to a workqueue.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 mm/alloc_tag.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 7481180dadd2..b80f5a151f28 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -591,6 +591,13 @@ void pgalloc_tag_swap(struct folio *new, struct folio *old)
 	put_page_tag_ref(handle_new);
 }
 
+static void remove_allocinfo_file(struct work_struct *work)
+{
+	remove_proc_entry(ALLOCINFO_FILE_NAME, NULL);
+}
+
+static DECLARE_WORK(remove_allocinfo_work, remove_allocinfo_file);
+
 static void shutdown_mem_profiling(bool remove_file)
 {
 	if (mem_alloc_profiling_enabled())
@@ -600,7 +607,7 @@ static void shutdown_mem_profiling(bool remove_file)
 		return;
 
 	if (remove_file)
-		remove_proc_entry(ALLOCINFO_FILE_NAME, NULL);
+		schedule_work(&remove_allocinfo_work);
 	mem_profiling_support = false;
 }
 
-- 
2.25.1