* [PATCH v5 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
@ 2026-08-12 5:41 Hao Ge
2026-08-12 5:41 ` [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
2026-08-12 5:41 ` [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
0 siblings, 2 replies; 6+ messages in thread
From: Hao Ge @ 2026-08-12 5:41 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 v5:
- add Fixes: and Cc: stable to patch 1/2 as well, since 2/2 does not
compile without it (Andrew Morton)
- restore frob-adjusted mem[type].size on retry instead of zeroing,
as s390 and parisc add GOT/PLT space there in
module_frob_arch_sections() (Reported by Sashiko)
- drop the load_module() mem_profiling_support check; the percpu
counter leak is pre-existing and orthogonal to this fix
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/
v4: https://lore.kernel.org/all/20260810093955.153015-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 | 25 ++++++++++-
mm/alloc_tag.c | 100 ++++++++++++++++++++++---------------------
2 files changed, 74 insertions(+), 51 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags()
2026-08-12 5:41 [PATCH v5 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
@ 2026-08-12 5:41 ` Hao Ge
2026-08-12 6:03 ` sashiko-bot
2026-08-12 5:41 ` [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
1 sibling, 1 reply; 6+ messages in thread
From: Hao Ge @ 2026-08-12 5:41 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
release_module_tags() is a cleanup helper. reserve_module_tags() can
also fail after storing the reservation in the maple tree, in which
case it should call release_module_tags() to undo it. Move the helper
above reserve_module_tags() so no forward declaration is needed.
No functional change.
Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Cc: stable@vger.kernel.org
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] 6+ messages in thread
* [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
2026-08-12 5:41 [PATCH v5 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-12 5:41 ` [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
@ 2026-08-12 5:41 ` Hao Ge
2026-08-12 6:04 ` sashiko-bot
1 sibling, 1 reply; 6+ messages in thread
From: Hao Ge @ 2026-08-12 5:41 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.
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 | 25 +++++++++++++++++++++++--
mm/alloc_tag.c | 8 +++++---
2 files changed, 28 insertions(+), 5 deletions(-)
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..ed26f167be84 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2944,6 +2944,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
{
struct module *mod;
int err;
+ unsigned long frob_size[MOD_MEM_NUM_TYPES];
/* Allow arches to frob section contents and sizes. */
err = module_frob_arch_sections(info->hdr, info->sechdrs,
@@ -2966,18 +2967,38 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
*/
module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
+ /*
+ * Save the sizes reserved by module_frob_arch_sections() so they can
+ * be restored if we retry below.
+ */
+ for_each_mod_mem_type(type)
+ frob_size[type] = info->mod->mem[type].size;
+
/*
* Determine total sizes, and put offsets in sh_entsize. For now
* this is done generically; there doesn't appear to be any
* special cases for the architectures.
*/
+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 = frob_size[type];
+ 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..0a7b657fe2de 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();
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags()
2026-08-12 5:41 ` [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
@ 2026-08-12 6:03 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-12 6: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] Missing cleanup in `reserve_module_tags()` error path leaves dangling pointer in maple tree.
- [High] `release_module_tags()` uses `mas_store()` which can fail silently under memory pressure, leaving a dangling pointer.
--
--- Patch [1]: [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() ---
commit 1dcbb4a041b40aefa702fd60db8fbd78ff223508
Author: Hao Ge <hao.ge@linux.dev>
alloc_tag: move release_module_tags() above reserve_module_tags()
This patch moves the release_module_tags() cleanup helper above
reserve_module_tags() to avoid the need for a forward declaration.
> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index b334103104779..2070e682fe106 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)
> +{
[ ... ]
> +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 is a pre-existing issue, but since the code is being moved here: could
mas_store() fail silently under memory pressure, leaving a dangling pointer?
When unloading a module, release_module_tags() calls mas_store() to clear
the module's reservation. If the internal maple tree allocation fails, is the
error state ignored? This could leave the tree retaining the old pointer,
which might lead to a use-after-free when the module subsystem frees it.
Would using mas_store_gfp(..., GFP_KERNEL) be needed here to ensure the
maple tree can sleep to allocate memory?
> +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 the commit message mentions
that reserve_module_tags() can fail after storing the reservation in the
maple tree and should call release_module_tags() to undo it.
However, looking at the existing reserve_module_tags() code down the line,
if vm_module_tags_populate() fails:
reserve_module_tags() {
...
grow_res = vm_module_tags_populate();
if (grow_res) {
shutdown_mem_profiling(true);
pr_err("Failed to allocate memory...\n", mod->name);
return ERR_PTR(grow_res);
}
...
}
Does this error path still return without actually calling
release_module_tags(mod, false)?
If the undo call is still missing here, it appears the maple tree mod_area_mt
might retain a pointer to the module that is being freed, potentially leading
to a use-after-free during subsequent lookups. Should that call be added here
in a follow-up patch?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812054105.102637-1-hao.ge@linux.dev?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
2026-08-12 5:41 ` [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
@ 2026-08-12 6:04 ` sashiko-bot
2026-08-12 6:32 ` Hao Ge
0 siblings, 1 reply; 6+ messages in thread
From: sashiko-bot @ 2026-08-12 6:04 UTC (permalink / raw)
To: Hao Ge; +Cc: linux-modules
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Unconditional allocation of per-CPU counters for modules causes a memory leak when memory profiling is disabled and the module load is retried.
Pre-existing issues:
- [Critical] Concurrent module loading can cause an out-of-bounds memory write and module corruption because the global profiling flag is toggled without cross-phase synchronization.
- [High] A race condition on the global module tags size variable can lead to unmapped virtual memory access and a kernel panic.
- [High] If page population fails, a dangling module pointer is left in the maple tree, leading to a Use-After-Free.
--
--- Patch [2]: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled ---
commit ffb2b208763f32cdfb3d6cb740d4d4efa5c99ae0
Author: Hao Ge <hao.ge@linux.dev>
alloc_tag: fix undetected compressed tag overflow when profiling is disabled
This patch modifies module layout and allocation to retry loading without
memory profiling if the compressed tag limit is exceeded. It removes the
profiling enabled guard when checking for tag overflow and handles the
fallback by resetting section flags and repeating the layout process.
> diff --git a/kernel/module/main.c b/kernel/module/main.c
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c
[ ... ]
> @@ -2966,18 +2967,38 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
> */
> module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
>
> + /*
> + * Save the sizes reserved by module_frob_arch_sections() so they can
> + * be restored if we retry below.
> + */
> + for_each_mod_mem_type(type)
> + frob_size[type] = info->mod->mem[type].size;
> +
> /*
> * Determine total sizes, and put offsets in sh_entsize. For now
> * this is done generically; there doesn't appear to be any
> * special cases for the architectures.
> */
> +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: Critical]
This isn't a bug introduced by this patch, but can concurrent module loading
cause memory corruption if profiling is disabled mid-flight?
In move_module(), if one thread disables profiling globally via
shutdown_mem_profiling() while another thread is halfway through module
loading (between layout calculation and memory movement), the second thread's
section offset queries could evaluate to false.
This would cause the module loader to use an un-incremented 0 offset and
incorrectly write the codetag section into the base memory, overwriting the
start of the module's core memory block.
> - 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 = frob_size[type];
> + 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]
Does this retry mechanism cause a memory leak for per-CPU counters?
When the module load is retried via this new loop, the codetag section is
treated as standard memory and bypasses maple tree registration. However, the
subsequent codetag module initialization unconditionally allocates per-CPU
memory for these tags regardless of the profiling state.
During module unload in release_module_tags(), the cleanup path exits early
because the module cannot be found in the maple tree, permanently leaking
the per-CPU memory on every unload.
> + }
>
> /* 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
> --- 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;
[Severity: High]
This is a pre-existing issue, but does the update to module_tags.size race
with concurrent module loads?
The high-water mark for the module tags size is updated here without a lock
after the maple tree lock is released. If two concurrent module loads race on
this update, a higher offset might be overwritten by a smaller one.
Subsequently, vm_module_tags_populate() relies on this corrupted size and
would fail to map the physical pages for the higher virtual addresses, leading
to a page fault when the module accesses its allocated tags.
> - 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();
[Severity: High]
This is a pre-existing issue, but could this lead to a dangling pointer in
the maple tree if page population fails?
If vm_module_tags_populate() fails and returns an error, the allocation
routine returns an error here but doesn't remove the newly inserted module
pointer from the maple tree.
Since this error bypasses the standard codetag cleanup routines in the module
loader (where codetag_section_found might remain false in move_module()),
the maple tree retains a pointer to the aborted module. Once the module
structure is freed, future maple tree iterations could dereference this freed
pointer.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260812054105.102637-1-hao.ge@linux.dev?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
2026-08-12 6:04 ` sashiko-bot
@ 2026-08-12 6:32 ` Hao Ge
0 siblings, 0 replies; 6+ messages in thread
From: Hao Ge @ 2026-08-12 6:32 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-modules, Andrew Morton, Suren Baghdasaryan
CC Andrew and Suren
On 2026/8/12 14:04, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
>
> New issues:
> - [High] Unconditional allocation of per-CPU counters for modules causes a memory leak when memory profiling is disabled and the module load is retried.
>
> Pre-existing issues:
> - [Critical] Concurrent module loading can cause an out-of-bounds memory write and module corruption because the global profiling flag is toggled without cross-phase synchronization.
> - [High] A race condition on the global module tags size variable can lead to unmapped virtual memory access and a kernel panic.
> - [High] If page population fails, a dangling module pointer is left in the maple tree, leading to a Use-After-Free.
> --
>
> --- Patch [2]: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled ---
> commit ffb2b208763f32cdfb3d6cb740d4d4efa5c99ae0
> Author: Hao Ge <hao.ge@linux.dev>
>
> alloc_tag: fix undetected compressed tag overflow when profiling is disabled
>
> This patch modifies module layout and allocation to retry loading without
> memory profiling if the compressed tag limit is exceeded. It removes the
> profiling enabled guard when checking for tag overflow and handles the
> fallback by resetting section flags and repeating the layout process.
>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>
> [ ... ]
>
>> @@ -2966,18 +2967,38 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>> */
>> module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
>>
>> + /*
>> + * Save the sizes reserved by module_frob_arch_sections() so they can
>> + * be restored if we retry below.
>> + */
>> + for_each_mod_mem_type(type)
>> + frob_size[type] = info->mod->mem[type].size;
>> +
>> /*
>> * Determine total sizes, and put offsets in sh_entsize. For now
>> * this is done generically; there doesn't appear to be any
>> * special cases for the architectures.
>> */
>> +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: Critical]
> This isn't a bug introduced by this patch, but can concurrent module loading
> cause memory corruption if profiling is disabled mid-flight?
>
> In move_module(), if one thread disables profiling globally via
> shutdown_mem_profiling() while another thread is halfway through module
> loading (between layout calculation and memory movement), the second thread's
> section offset queries could evaluate to false.
>
> This would cause the module loader to use an un-incremented 0 offset and
> incorrectly write the codetag section into the base memory, overwriting the
> start of the module's core memory block.
>
>> - 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 = frob_size[type];
>> + 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]
> Does this retry mechanism cause a memory leak for per-CPU counters?
>
> When the module load is retried via this new loop, the codetag section is
> treated as standard memory and bypasses maple tree registration. However, the
> subsequent codetag module initialization unconditionally allocates per-CPU
> memory for these tags regardless of the profiling state.
>
> During module unload in release_module_tags(), the cleanup path exits early
> because the module cannot be found in the maple tree, permanently leaking
> the per-CPU memory on every unload.
The per-CPU counter leak is pre-existing, not introduced by the retry
mechanism.
The root cause is that shutdown_mem_profiling() permanently sets
mem_profiling_support to false, after which needs_section_mem()
returns false
for every subsequent module. Their codetag sections are
placed as regular data, so they
are never inserted into the alloc_tag
maple tree. Yet codetag_load_module() still
unconditionally invokes
the load_module() callback, which allocates percpu counters
that
release_module_tags() cannot find on unload.
I'll send out a separate patch to fix this leak later.
>
>> + }
>>
>> /* 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
>> --- 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;
>
> [Severity: High]
> This is a pre-existing issue, but does the update to module_tags.size race
> with concurrent module loads?
>
> The high-water mark for the module tags size is updated here without a lock
> after the maple tree lock is released. If two concurrent module loads race on
> this update, a higher offset might be overwritten by a smaller one.
>
> Subsequently, vm_module_tags_populate() relies on this corrupted size and
> would fail to map the physical pages for the higher virtual addresses, leading
> to a page fault when the module accesses its allocated tags.
>
>> - 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();
>
> [Severity: High]
> This is a pre-existing issue, but could this lead to a dangling pointer in
> the maple tree if page population fails?
>
> If vm_module_tags_populate() fails and returns an error, the allocation
> routine returns an error here but doesn't remove the newly inserted module
> pointer from the maple tree.
>
> Since this error bypasses the standard codetag cleanup routines in the module
> loader (where codetag_section_found might remain false in move_module()),
> the maple tree retains a pointer to the aborted module. Once the module
> structure is freed, future maple tree iterations could dereference this freed
> pointer.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-12 6:32 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 5:41 [PATCH v5 0/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-12 5:41 ` [PATCH v5 1/2] alloc_tag: move release_module_tags() above reserve_module_tags() Hao Ge
2026-08-12 6:03 ` sashiko-bot
2026-08-12 5:41 ` [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled Hao Ge
2026-08-12 6:04 ` sashiko-bot
2026-08-12 6:32 ` 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).