Linux Modules
 help / color / mirror / Atom feed
* [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
@ 2026-08-10  9:39 Hao Ge
  2026-08-10  9:39 ` [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Hao Ge @ 2026-08-10  9:39 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

v3 was a single patch. After discussion with Suren and Andrew we went
for a more graceful approach: rather than failing the module load on
overflow, let it load without profiling. Once profiling is disabled,
codetag_needs_module_section() returns false, so on retry the codetag
section is placed as regular module data.

A new patch (1/2) is added to move release_module_tags() above
reserve_module_tags(), since the overflow path now has to call it and
the helper sits below it.

release_module_tags() is what module unload calls to drop a module's
reservation from the maple tree. By the time reserve_module_tags()
detects the overflow it has already stored that reservation, and the
-EAGAIN return skips vm_module_tags_populate(), so the backing pages
never get mapped. If reserve_module_tags() returns without calling
release_module_tags(), the stale entry keeps pointing at that unmapped
range; when the module is later unloaded, release_module_tags() walks
it and panics.

Tested on an x86_64 virtual machine:

# insmod overflow_tag.ko
# dmesg
  With module overflow_tag there are too many tags to fit in 13 page
  flag bits. Memory allocation profiling is disabled!
# rmmod overflow_tag
The module loads without profiling.

Changes in v4:
- add a new patch (1/2) to move release_module_tags() above
  reserve_module_tags(); the overflow fix is 2/2
- release the reservation on the -EAGAIN path
- return -EAGAIN instead of -ENOMEM so the module can still load
  without profiling (Suren)
- reset sh_addr, mem[type].size and sym/str SHF_ALLOC before retry
- skip percpu counters in load_module() when profiling is off

Changes in v3:
- use pr_warn_once() instead of pr_warn()
- return -ENOMEM instead of -ENOSPC (Suren)
- expand the commit message to describe the /proc/allocinfo impact
  (Andrew)

Changes in v2:
- return an error after shutdown_mem_profiling() to skip
  vm_module_tags_populate()

v1: https://lore.kernel.org/all/20260804064408.105033-1-hao.ge@linux.dev/
v2: https://lore.kernel.org/all/20260804122038.190270-1-hao.ge@linux.dev/
v3: https://lore.kernel.org/all/20260805090633.141001-1-hao.ge@linux.dev/

Hao Ge (2):
  alloc_tag: move release_module_tags() above reserve_module_tags()
  alloc_tag: fix undetected compressed tag overflow when profiling is
    disabled

 kernel/module/main.c |  17 ++++++++-
 mm/alloc_tag.c       | 103 +++++++++++++++++++++++++++------------------------
 2 files changed, 69 insertions(+), 51 deletions(-)

-- 
2.25.1

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

* [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags()
  2026-08-10  9:39 [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
@ 2026-08-10  9:39 ` Hao Ge
  2026-08-10 10:03   ` sashiko-bot
  2026-08-10  9:39 ` [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
  2026-08-11  3:52 ` [PATCH v4 0/2] " Andrew Morton
  2 siblings, 1 reply; 7+ messages in thread
From: Hao Ge @ 2026-08-10  9:39 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

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.

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 52aece27b00e..af44f90379f2 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -835,6 +835,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)
 {
@@ -922,52 +968,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


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

* [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
  2026-08-10  9:39 [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
  2026-08-10  9:39 ` [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
@ 2026-08-10  9:39 ` Hao Ge
  2026-08-10 10:03   ` sashiko-bot
  2026-08-11  3:52 ` [PATCH v4 0/2] " Andrew Morton
  2 siblings, 1 reply; 7+ messages in thread
From: Hao Ge @ 2026-08-10  9:39 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, stable

In reserve_module_tags(), the tag overflow check is gated on
mem_alloc_profiling_enabled():

    if (mem_alloc_profiling_enabled() && !tags_addressable())

If profiling is toggled off at runtime and a module is loaded whose
tags exceed the compressed-mode limit, shutdown_mem_profiling() is
skipped. vm_module_tags_populate() still maps memory for the tags and
the module loads successfully, but the total tag count now exceeds what
NR_UNUSED_PAGEFLAG_BITS can address.

Once profiling is re-enabled, ref_to_idx() computes each tag's index
as its position in the alloc_tag array. update_page_tag_ref() masks
it to alloc_tag_ref_mask before storing in page->flags. Indices
beyond the mask are truncated and idx_to_ref() resolves them to wrong
tags.

This silently corrupts /proc/allocinfo: allocated pages get attributed
to the wrong call sites, so the statistics it reports are wrong.

mem_alloc_profiling_enabled() and mem_profiling_compressed are
independent. Once compressed mode is established at boot, it stays
active regardless of runtime toggles of mem_profiling.

Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down
profiling, release the reservation, and return -EAGAIN so that
layout_and_allocate() retries with profiling disabled: codetag sections
are then placed as regular module data and the module loads without
profiling rather than being rejected entirely.

Skip percpu counter allocation in load_module() when profiling is
disabled, as those counters would never be freed on unload.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: stable@vger.kernel.org
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 kernel/module/main.c | 17 +++++++++++++++--
 mm/alloc_tag.c       | 11 ++++++++---
 2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..c32bb47a573a 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2971,13 +2971,26 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
 	 * this is done generically; there doesn't appear to be any
 	 * special cases for the architectures.
 	 */
+retry:
 	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)
-		return ERR_PTR(err);
+	if (err) {
+		if (err != -EAGAIN)
+			return ERR_PTR(err);
+		/*
+		 * -EAGAIN means profiling was disabled but the module
+		 * can still load without it. Reset state and retry.
+		 */
+		rewrite_section_headers(info, flags);
+		for_each_mod_mem_type(type)
+			info->mod->mem[type].size = 0;
+		info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
+		info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
+		goto retry;
+	}
 
 	/* 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 af44f90379f2..3eba6331174d 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c
@@ -950,10 +950,12 @@ 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);
+			return ERR_PTR(-EAGAIN);
 		}
 
 		grow_res = vm_module_tags_populate();
@@ -975,6 +977,9 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
 	struct alloc_tag *stop_tag;
 	struct alloc_tag *tag;
 
+	if (!mem_profiling_support)
+		return 0;
+
 	/* percpu counters for core allocations are already statically allocated */
 	if (!mod)
 		return 0;
-- 
2.25.1


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

* Re: [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags()
  2026-08-10  9:39 ` [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
@ 2026-08-10 10:03   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-10 10:03 UTC (permalink / raw)
  To: Hao Ge; +Cc: linux-modules

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

Pre-existing issues:
- [High] reserve_module_tags() silently succeeds and returns a valid-looking pointer when mas_store() fails, corrupting the module tags allocation space.
- [High] release_module_tags() ignores failures from mas_store(), permanently leaking the module's maple tree range.
--

--- Patch [1]: [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() ---
commit 96b4c3fa8241c380a13e2a15009ac553cfc71daa
Author: Hao Ge <hao.ge@linux.dev>

alloc_tag: move release_module_tags() above reserve_module_tags()

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.

> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index e93e7fec1f066..96a9196635fea 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -493,6 +493,52 @@ static int vm_module_tags_populate(void)
>  	return 0;
>  }
>  
> +static void release_module_tags(struct module *mod, bool used)
> +{

[ ... ]

> +release_area:
> +	mas_store(&mas, used ? &unloaded_mod : NULL);
> +	val = mas_prev_range(&mas, 0);
> +	if (val == &prepend_mod)
> +		mas_store(&mas, NULL);

[Severity: High]
This isn't a bug introduced by this patch, but does this ignore failures from
mas_store()?

Since mas_store() is called with the maple tree lock held, it forces an
implicit GFP_NOWAIT allocation. If this allocation fails under memory
pressure, mas_store() sets mas into an error state without modifying the tree.

Because the return value is ignored, the next call mas_prev_range(&mas, 0)
would immediately return NULL upon seeing the error state, causing the padding
cleanup mas_store(&mas, NULL) to be skipped.

Could this result in the module's maple tree range being permanently leaked
because the original module pointer remains in the tree?

> +out:
> +	mas_unlock(&mas);
> +}
> +
> +static void *reserve_module_tags(struct module *mod, unsigned long size,
> +				 unsigned int prepend, unsigned long align)
> +{

[Severity: High]
This isn't a bug introduced by this patch, but does reserve_module_tags()
silently succeed when mas_store() fails?

If mas_store(&mas, mod) fails due to memory pressure:

	if (mas_is_err(&mas)) {
		mas.index = pad_start;
		mas_erase(&mas);
		ret = ERR_PTR(xa_err(mas.node));
	}

Does mas_erase(&mas) destroy the error state in mas.node before it can be
read?

mas_erase() resets the maple state and walks the tree, overwriting the error
pointer in mas.node with a valid tree node or NULL. The subsequent
ret = ERR_PTR(xa_err(mas.node)) then reads this non-error node, resulting in
ret = NULL.

Since ret is not an error pointer, would IS_ERR(ret) evaluate to false,
causing the function to incorrectly return success?

[ ... ]

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

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

* Re: [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
  2026-08-10  9:39 ` [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
@ 2026-08-10 10:03   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-10 10:03 UTC (permalink / raw)
  To: Hao Ge; +Cc: linux-modules

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

New issues:
- [High] Skipping `tag->counters` allocation leads to invalid memory access (garbage reads or crashes) when reading tags.
- [High] The retry loop skips `module_frob_arch_sections()`, causing module memory corruption on architectures that allocate space inside it.

Pre-existing issues:
- [High] Concurrent module loading can lead to silent module memory corruption if allocation profiling is dynamically disabled.
--

--- Patch [2]: [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled ---
commit 9e076bfe2df23289b04f8850ce26739d08894c81
Author: Hao Ge <hao.ge@linux.dev>

alloc_tag: fix undetected compressed tag overflow when profiling is disabled

This commit removes the mem_alloc_profiling_enabled() guard when checking for
tag overflows in reserve_module_tags(). If an overflow occurs, it shuts down
profiling and uses an -EAGAIN retry loop in layout_and_allocate() to load the
module without profiling. It also skips allocating per-cpu counters if profiling
is disabled.

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 46dd8d25a6058..c32bb47a573a1 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -2971,13 +2971,26 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>  	 * this is done generically; there doesn't appear to be any
>  	 * special cases for the architectures.
>  	 */
> +retry:
>  	layout_sections(info->mod, info);
>  	layout_symtab(info->mod, info);
>  
>  	/* Allocate and move to the final place */
>  	err = move_module(info->mod, info);

[Severity: High]
This isn't a bug introduced by this patch, but could concurrent module loading
lead to silent memory corruption if allocation profiling is dynamically
disabled?

When two modules load concurrently, Module A might calculate section offsets
in layout_sections() while mem_profiling_support is true. Its .alloc_tags
section gets a 0 offset since codetag_needs_module_section() evaluates to
true.

Concurrently, Module B might trigger a tag overflow in reserve_module_tags(),
which calls shutdown_mem_profiling() and sets mem_profiling_support to false
locklessly.

When Module A reaches move_module(), codetag_needs_module_section() now
evaluates to false. Does this cause the fallback path to place the section
at offset 0 within the module's core memory block, overwriting the first
valid section?

> -	if (err)
> -		return ERR_PTR(err);
> +	if (err) {
> +		if (err != -EAGAIN)
> +			return ERR_PTR(err);
> +		/*
> +		 * -EAGAIN means profiling was disabled but the module
> +		 * can still load without it. Reset state and retry.
> +		 */
> +		rewrite_section_headers(info, flags);
> +		for_each_mod_mem_type(type)
> +			info->mod->mem[type].size = 0;
> +		info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
> +		info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
> +		goto retry;

[Severity: High]
Since the retry loop zeroes mod->mem[type].size and jumps to the retry label,
does it skip module_frob_arch_sections() which was called earlier in
layout_and_allocate()?

Architectures like s390 use module_frob_arch_sections() to reserve space at
the beginning of MOD_TEXT by modifying mod->mem[MOD_TEXT].size and caching
offsets:

arch/s390/kernel/module.c:module_frob_arch_sections() {
    ...
    mod_mem = &me->mem[MOD_TEXT];
    mod_mem->size = ALIGN(mod_mem->size, 4);
    me->arch.got_offset = mod_mem->size;
    mod_mem->size += me->arch.got_size;
    ...
}

Will skipping this initialization on retry cause the architecture loader to
place normal ELF sections at offset 0, and then overwrite the executable
.text section later during relocation using the cached offsets?

> +	}
>  
>  	/* 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 96a9196635fea..3c81ec7988860 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
[ ... ]
> @@ -633,6 +635,9 @@ static int load_module(struct module *mod, struct codetag *start, struct codetag
>  	struct alloc_tag *stop_tag;
>  	struct alloc_tag *tag;
>  
> +	if (!mem_profiling_support)
> +		return 0;

[Severity: High]
If we return 0 here, does it skip allocating tag->counters while still
allowing the module to load with its .alloc_tags section processed as
regular data?

Later, when show_mem() is triggered or /proc/allocinfo is read,
alloc_tag_top_users() unconditionally iterates over the global tags:

mm/alloc_tag.c:alloc_tag_top_users() {
    ...
    while ((ct = codetag_next_ct(&iter))) {
        struct alloc_tag_counters counter = alloc_tag_read(ct_to_alloc_tag(ct));
    ...
}

Will calling alloc_tag_read() dereference the NULL counter?

include/linux/alloc_tag.h:alloc_tag_read() {
    ...
    counter = per_cpu_ptr(tag->counters, cpu);
    v.bytes += counter->bytes;
    ...
}

Because per_cpu_ptr doesn't check for NULL, won't this add the per-cpu
offset to 0 and attempt to read unmapped memory or garbage data, causing a
kernel crash?

> +
>  	/* percpu counters for core allocations are already statically allocated */
>  	if (!mod)
>  		return 0;

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

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

* Re: [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
  2026-08-10  9:39 [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
  2026-08-10  9:39 ` [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
  2026-08-10  9:39 ` [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
@ 2026-08-11  3:52 ` Andrew Morton
  2026-08-11 15:35   ` Suren Baghdasaryan
  2 siblings, 1 reply; 7+ messages in thread
From: Andrew Morton @ 2026-08-11  3:52 UTC (permalink / raw)
  To: Hao Ge
  Cc: Suren Baghdasaryan, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
	Sami Tolvanen, Aaron Tomlin, linux-modules, linux-kernel,
	linux-mm

On Mon, 10 Aug 2026 17:39:53 +0800 Hao Ge <hao.ge@linux.dev> wrote:

> v3 was a single patch. After discussion with Suren and Andrew we went
> for a more graceful approach: rather than failing the module load on
> overflow, let it load without profiling. Once profiling is disabled,
> codetag_needs_module_section() returns false, so on retry the codetag
> section is placed as regular module data.
> 
> A new patch (1/2) is added to move release_module_tags() above
> reserve_module_tags(), since the overflow path now has to call it and
> the helper sits below it.

Thing is, [2/2] has cc:stable but it requires [1/2] to be able to be
compiled.  [1/2] doesn't have cc:stable so we're asking -stable folks
to backport a patch which doesn't compile.

Resolve this by using the same Fixes: and cc:stable in both patches.

> release_module_tags() is what module unload calls to drop a module's
> reservation from the maple tree. By the time reserve_module_tags()
> detects the overflow it has already stored that reservation, and the
> -EAGAIN return skips vm_module_tags_populate(), so the backing pages
> never get mapped. If reserve_module_tags() returns without calling
> release_module_tags(), the stale entry keeps pointing at that unmapped
> range; when the module is later unloaded, release_module_tags() walks
> it and panics.

AI review had a lot to say about this patchset.  Some pre-existing, some
not:
	https://sashiko.dev/#/patchset/20260810093955.153015-1-hao.ge@linux.dev



offtopic: alloc_tag isn't getting allmodconfig build coverage at this
time because:

1: MEM_ALLOC_PROFILING depends on !DEBUG_FORCE_WEAK_PER_CPU (why?  I
   can't figure that out)

2: x86_64 allmodconfig enables DEBUG_FORCE_WEAK_PER_CPU, despite it
   being for s390 and alpha.  In fact it might be alpha-only.

Adding

	depends on ALPHA || S390

in there fixes this.

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

* Re: [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
  2026-08-11  3:52 ` [PATCH v4 0/2] " Andrew Morton
@ 2026-08-11 15:35   ` Suren Baghdasaryan
  0 siblings, 0 replies; 7+ messages in thread
From: Suren Baghdasaryan @ 2026-08-11 15:35 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Hao Ge, Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
	Aaron Tomlin, linux-modules, linux-kernel, linux-mm

On Mon, Aug 10, 2026 at 8:52 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon, 10 Aug 2026 17:39:53 +0800 Hao Ge <hao.ge@linux.dev> wrote:
>
> > v3 was a single patch. After discussion with Suren and Andrew we went
> > for a more graceful approach: rather than failing the module load on
> > overflow, let it load without profiling. Once profiling is disabled,
> > codetag_needs_module_section() returns false, so on retry the codetag
> > section is placed as regular module data.
> >
> > A new patch (1/2) is added to move release_module_tags() above
> > reserve_module_tags(), since the overflow path now has to call it and
> > the helper sits below it.
>
> Thing is, [2/2] has cc:stable but it requires [1/2] to be able to be
> compiled.  [1/2] doesn't have cc:stable so we're asking -stable folks
> to backport a patch which doesn't compile.
>
> Resolve this by using the same Fixes: and cc:stable in both patches.
>
> > release_module_tags() is what module unload calls to drop a module's
> > reservation from the maple tree. By the time reserve_module_tags()
> > detects the overflow it has already stored that reservation, and the
> > -EAGAIN return skips vm_module_tags_populate(), so the backing pages
> > never get mapped. If reserve_module_tags() returns without calling
> > release_module_tags(), the stale entry keeps pointing at that unmapped
> > range; when the module is later unloaded, release_module_tags() walks
> > it and panics.
>
> AI review had a lot to say about this patchset.  Some pre-existing, some
> not:
>         https://sashiko.dev/#/patchset/20260810093955.153015-1-hao.ge@linux.dev

Yeah, some of them are not related to this change but at least one does.
I need to address the unrelated ones. Will do that as a separate patchset.

>
>
>
> offtopic: alloc_tag isn't getting allmodconfig build coverage at this
> time because:
>
> 1: MEM_ALLOC_PROFILING depends on !DEBUG_FORCE_WEAK_PER_CPU (why?  I
>    can't figure that out)

DEBUG_FORCE_WEAK_PER_CPU forces weak percpu definitions everywhere,
even in the core kernel. This introduces the restriction of [1]:

2. Static percpu variables cannot be defined inside a function.

Memory allocation profiling relies on percpu variables inside a
function in DEFINE_ALLOC_TAG(). For
CONFIG_ARCH_MODULE_NEEDS_WEAK_PER_CPU we comporomise by accounting all
module allocations to a statically defined _shared_alloc_tag (see [2])
but we can't do that for all kernel allocations because profiling
becomes quite meaningless at that point (all allocations being
accounted in the same counter is not useful).

[1] https://elixir.bootlin.com/linux/v7.2-rc6/source/include/linux/percpu-defs.h#L64
[2]  https://elixir.bootlin.com/linux/v7.2-rc6/source/include/linux/alloc_tag.h#L91

>
> 2: x86_64 allmodconfig enables DEBUG_FORCE_WEAK_PER_CPU, despite it
>    being for s390 and alpha.  In fact it might be alpha-only.
>
> Adding
>
>         depends on ALPHA || S390
>
> in there fixes this.

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

end of thread, other threads:[~2026-08-11 15:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-10  9:39 [PATCH v4 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-10  9:39 ` [PATCH v4 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
2026-08-10 10:03   ` sashiko-bot
2026-08-10  9:39 ` [PATCH v4 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-10 10:03   ` sashiko-bot
2026-08-11  3:52 ` [PATCH v4 0/2] " Andrew Morton
2026-08-11 15:35   ` Suren Baghdasaryan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox