[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing error code assignment when codetag_load_module() fails in load_module().
- [High] Use-After-Free and memory leak due to missing codetag_unload_module() on the module load error path.
--

--- Patch [1]: [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled ---
commit 355b88475bea314f9aab64a9aa73da39a0d2bcf0
Author: Hao Ge <hao.ge@linux.dev>

alloc_tag: skip percpu counter allocation when profiling is disabled

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.
This patch returns CODETAG_MODULE_EXCLUDED when profiling is off to skip
percpu counter allocation.

> diff --git a/lib/codetag.c b/lib/codetag.c
> index a9cda4c962a30..8506ecab9ea70 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;

[Severity: High]
This is a pre-existing issue, but if codetag_module_init() fails midway through
the codetag types, does it leave the module registered in the previously
initialized types?

lib/codetag.c:codetag_load_module() {
...
	list_for_each_entry(cttype, &codetag_types, link) {
		ret = codetag_module_init(cttype, mod);
		if (ret)
			break;
	}
...
}

If the loop breaks on an error, the successfully initialized types are not
unwound. When load_module() in kernel/module/main.c handles this error, it
jumps to sysfs_cleanup and eventually frees the module, but misses a call to
codetag_unload_module(mod). 

Could this lead to a use-after-free or a memory leak when traversing the IDR
later, since the freed module is still registered?


[Severity: High]
This is another pre-existing issue. Is the error code lost in load_module()
when codetag_load_module() fails?

kernel/module/main.c:load_module() {
...
	if (codetag_load_module(mod))
		goto sysfs_cleanup;
...
}

Since err is not assigned before jumping to sysfs_cleanup, would the syscall
incorrectly return 0 (success) even though the module failed to load and was
destroyed on the error path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] An integer overflow in ALIGN() using unvalidated sh_addralign allows an attacker to spoof the memory type stored in sh_entsize, leading to an out-of-bounds read and kernel panic.
--

--- Patch [2]: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() ---
commit f621cb3d3b6809c9dd569a1f5e20bfb1e929c4d4
Author: Hao Ge <hao.ge@linux.dev>

module: move codetag section placement decision to layout_sections()

This commit moves the decision for codetag section placement to
layout_sections(), storing the result in sh_entsize using the new
MOD_MEM_CODETAG type. The move_module() function now relies on this stored
type instead of calling codetag_needs_module_section() again. This prevents
a race condition when profiling support is toggled during module load.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a6058..25056e87d0038 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -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) {

[Severity: High]
Can a maliciously crafted ELF module spoof the type by causing an integer
overflow in module_get_offset_and_type()?

If a module with two SHT_NOBITS sections is loaded, it might bypass ELF file
size validations. The first section could inflate mod->mem[MOD_DATA].size to
0x60000001. If the second section has sh_addralign = 0x10000000, it would
cause an integer overflow in module_get_offset_and_type():

kernel/module/main.c:module_get_offset_and_type() {
    ...
    offset = ALIGN(mod->mem[type].size, sechdr->sh_addralign ?: 1);
    ...
    return offset | mask;
}

The overflowed offset (0x70000000) places 7 (MOD_MEM_CODETAG) into the top 4
bits of sh_entsize. By setting the second section's sh_size to 0x90000000,
the attacker can wrap mod->mem[MOD_DATA].size back to 0, avoiding -ENOMEM
during module_memory_alloc().

If this spoofed type is trusted by move_module(), execution routes to
codetag_alloc_module_section():

lib/codetag.c:codetag_alloc_module_section() {
    ...
    const char *type_name = name + strlen(CODETAG_SECTION_PREFIX);
    ...
}

If a short-named section is routed here by the spoofed type, could blindly
advancing the name pointer cause an out-of-bounds read past the secstrings
vmalloc allocation and trigger a kernel panic?

>  			dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
>  					arch_mod_section_prepend(mod, i), shdr->sh_addralign);
>  			if (WARN_ON(!dest)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=2

[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] TOCTOU race in sysctl handler allows memory profiling to remain active without support structures.
--

--- Patch [3]: [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock ---
commit 8d4c5cdeb441b4a47f3511f205a68485820706b0
Author: Hao Ge <hao.ge@linux.dev>

alloc_tag: remove /proc/allocinfo outside of mod_lock

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(). Move remove_proc_entry() to a workqueue.

> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 26c6ebc96a6ae..65d0ae9af94b2 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c

[ ... ]

> @@ -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;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but does shutdown_mem_profiling()
have a TOCTOU race with the sysctl handler proc_mem_profiling_handler()?

If a concurrent module load fails setup and calls shutdown_mem_profiling(),
it disables the static key and sets mem_profiling_support = false.

Concurrently, could a sysctl write to /proc/sys/vm/mem_profiling evaluate
mem_profiling_support before it is set to false:

mm/alloc_tag.c:proc_mem_profiling_handler() {
    ...
        if (!mem_profiling_support)
            return -EINVAL;
    }

    return proc_do_static_key(table, write, buffer, lenp, ppos);
}

and then execute proc_do_static_key() after shutdown_mem_profiling() has
finished, errantly re-enabling the static key?

If the static key is enabled but mem_profiling_support is false, any
subsequent module load will skip allocating per-cpu tag counters. When
the new module executes a memory allocation, would alloc_tag_ref_set()
unconditionally dereference a NULL tag->counters?

include/linux/alloc_tag.h:alloc_tag_ref_set() {
    ...
    this_cpu_inc(tag->counters->calls);
    ...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=3