linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling()
@ 2026-08-13  9:34 Hao Ge
  2026-08-13  9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Hao Ge @ 2026-08-13  9:34 UTC (permalink / raw)
  To: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Aaron Tomlin
  Cc: linux-modules, linux-kernel, linux-mm, 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



^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled
  2026-08-13  9:34 [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling() Hao Ge
@ 2026-08-13  9:34 ` Hao Ge
  2026-08-15  6:15   ` Suren Baghdasaryan
  2026-08-13  9:34 ` [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() Hao Ge
  2026-08-13  9:34 ` [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
  2 siblings, 1 reply; 10+ messages in thread
From: Hao Ge @ 2026-08-13  9:34 UTC (permalink / raw)
  To: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Aaron Tomlin
  Cc: linux-modules, linux-kernel, linux-mm, 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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()
  2026-08-13  9:34 [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling() Hao Ge
  2026-08-13  9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
@ 2026-08-13  9:34 ` Hao Ge
  2026-08-15  6:21   ` Suren Baghdasaryan
  2026-08-13  9:34 ` [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
  2 siblings, 1 reply; 10+ messages in thread
From: Hao Ge @ 2026-08-13  9:34 UTC (permalink / raw)
  To: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Aaron Tomlin
  Cc: linux-modules, linux-kernel, linux-mm, 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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock
  2026-08-13  9:34 [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling() Hao Ge
  2026-08-13  9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
  2026-08-13  9:34 ` [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() Hao Ge
@ 2026-08-13  9:34 ` Hao Ge
  2026-08-15  6:51   ` Suren Baghdasaryan
  2 siblings, 1 reply; 10+ messages in thread
From: Hao Ge @ 2026-08-13  9:34 UTC (permalink / raw)
  To: Suren Baghdasaryan, Andrew Morton, Luis Chamberlain, Petr Pavlu,
	Daniel Gomez, Sami Tolvanen, Aaron Tomlin
  Cc: linux-modules, linux-kernel, linux-mm, 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



^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled
  2026-08-13  9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
@ 2026-08-15  6:15   ` Suren Baghdasaryan
  2026-08-17  5:05     ` Hao Ge
  0 siblings, 1 reply; 10+ messages in thread
From: Suren Baghdasaryan @ 2026-08-15  6:15 UTC (permalink / raw)
  To: Hao Ge
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>
> 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>

Thanks for the fix. I think it could be done simpler, see below.

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

I see no reason for adding these special values. You could simply
return -ENOTSUP when profiling is disabled.

> +
>  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. */

IIUC here you want to call kfree() if profiling got disabled. If you
return -ENOTSUP instead of CODETAG_MODULE_EXCLUDED then this condition
does not need to change.

>                 kfree(cmod);
> -               return err;
> +               return err < 0 ? err : 0;

Here you can do:
                    if (err && err != -ENOTSUP)
                            return err;
                   return 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;

Return -ENOTSUP here.



> +
>         /* 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
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()
  2026-08-13  9:34 ` [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() Hao Ge
@ 2026-08-15  6:21   ` Suren Baghdasaryan
  2026-08-17  5:37     ` Hao Ge
  0 siblings, 1 reply; 10+ messages in thread
From: Suren Baghdasaryan @ 2026-08-15  6:21 UTC (permalink / raw)
  To: Hao Ge
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>
> 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

Ok, it feels a bit hacky but it's probably the simplest way to mark
codetag regions.

> +
>  #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);

I think this requires your patch [1] from another patchset to work
correctly, correct? If so, I would suggest sending this patch as part
of that patchset since there is a dependency.

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


> +
>         /*
>          * 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
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock
  2026-08-13  9:34 ` [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
@ 2026-08-15  6:51   ` Suren Baghdasaryan
  2026-08-17  5:41     ` Hao Ge
  0 siblings, 1 reply; 10+ messages in thread
From: Suren Baghdasaryan @ 2026-08-15  6:51 UTC (permalink / raw)
  To: Hao Ge
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>
> 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

Yes, this is indeed a possible race.

>
> Move remove_proc_entry() to a workqueue.
>
> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")

I think all these issues were reported by Sashiko, so you should add a
Reported-by tag.

> Signed-off-by: Hao Ge <hao.ge@linux.dev>

LGTM

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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled
  2026-08-15  6:15   ` Suren Baghdasaryan
@ 2026-08-17  5:05     ` Hao Ge
  0 siblings, 0 replies; 10+ messages in thread
From: Hao Ge @ 2026-08-17  5:05 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

Hi Suren


On 2026/8/15 14:15, Suren Baghdasaryan wrote:
> On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>>
>> 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>
> 
> Thanks for the fix. I think it could be done simpler, see below.

Agree, will do in the next version.

> 
>> ---
>>  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 */
> 
> I see no reason for adding these special values. You could simply
> return -ENOTSUP when profiling is disabled.
> 
>> +
>>  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. */
> 
> IIUC here you want to call kfree() if profiling got disabled. If you
> return -ENOTSUP instead of CODETAG_MODULE_EXCLUDED then this condition
> does not need to change.
> 
>>                 kfree(cmod);
>> -               return err;
>> +               return err < 0 ? err : 0;
> 
> Here you can do:
>                     if (err && err != -ENOTSUP)
>                             return err;
>                    return 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;
> 
> Return -ENOTSUP here.
> 
> 
> 
>> +
>>         /* 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
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections()
  2026-08-15  6:21   ` Suren Baghdasaryan
@ 2026-08-17  5:37     ` Hao Ge
  0 siblings, 0 replies; 10+ messages in thread
From: Hao Ge @ 2026-08-17  5:37 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

Hi Suren

Thanks for your review.

On 2026/8/15 14:21, Suren Baghdasaryan wrote:
> On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>> 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
> Ok, it feels a bit hacky but it's probably the simplest way to mark
> codetag regions.
> 

Yeah, it's not the prettiest — that's part of why I sent this series
as an RFC. Actually, I was wondering whether we could turn this into a
generic marker which means the section does not live in mod->mem[] but
in a region managed by its owning subsystem, codetag being the first
user. Of course, we still need to consult the module subsystem experts
about this.

>> +
>>  #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);
> I think this requires your patch [1] from another patchset to work
> correctly, correct? If so, I would suggest sending this patch as part
> of that patchset since there is a dependency.
> 

Right, this hunk needs the retry from [1] to work. I'll drop this
patch from this series and send it as part of that patchset instead.

I'll wait until we sort out the plan for [1] with Petr before pushing this.
Feedback from the module‑subsystem experts on this patch is most welcome.

> [1] https://lore.kernel.org/all/20260812054105.102637-3-hao.ge@linux.dev/
>
> 
>> +
>>         /*
>>          * 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock
  2026-08-15  6:51   ` Suren Baghdasaryan
@ 2026-08-17  5:41     ` Hao Ge
  0 siblings, 0 replies; 10+ messages in thread
From: Hao Ge @ 2026-08-17  5:41 UTC (permalink / raw)
  To: Suren Baghdasaryan
  Cc: Andrew Morton, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

Hi Suren

On 2026/8/15 14:51, Suren Baghdasaryan wrote:
> On Thu, Aug 13, 2026 at 2:34 AM Hao Ge <hao.ge@linux.dev> wrote:
>>
>> 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
> 
> Yes, this is indeed a possible race.
> 
>>
>> Move remove_proc_entry() to a workqueue.
>>
>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> 
> I think all these issues were reported by Sashiko, so you should add a
> Reported-by tag.
> 

Right, I'll add them. Patches 1 and 3 will be resent as a standalone
two-patch series with the Reported-by tags included.
Thanks a lot for all the help on this series.

Thanks
Best Regards
Hao

>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> 
> LGTM
> 
>> ---
>>  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
>>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-17  5:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13  9:34 [RFC PATCH 0/3] alloc_tag: fix races and a leak around shutdown_mem_profiling() Hao Ge
2026-08-13  9:34 ` [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled Hao Ge
2026-08-15  6:15   ` Suren Baghdasaryan
2026-08-17  5:05     ` Hao Ge
2026-08-13  9:34 ` [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() Hao Ge
2026-08-15  6:21   ` Suren Baghdasaryan
2026-08-17  5:37     ` Hao Ge
2026-08-13  9:34 ` [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock Hao Ge
2026-08-15  6:51   ` Suren Baghdasaryan
2026-08-17  5:41     ` Hao Ge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).