* [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; 13+ 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] 13+ 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-13 9:49 ` sashiko-bot 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, 2 replies; 13+ 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] 13+ 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-13 9:49 ` sashiko-bot 2026-08-15 6:15 ` Suren Baghdasaryan 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-08-13 9:49 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 error code assignment when codetag_load_module() fails in load_module(). - [High] Use-After-Free and memory leak due to missing codetag_unload_module() on the module load error path. -- --- Patch [1]: [RFC PATCH 1/3] alloc_tag: skip percpu counter allocation when profiling is disabled --- commit 355b88475bea314f9aab64a9aa73da39a0d2bcf0 Author: Hao Ge <hao.ge@linux.dev> alloc_tag: skip percpu counter allocation when profiling is disabled After shutdown_mem_profiling() clears mem_profiling_support, needs_section_mem() returns false, so later modules have their codetag section placed as regular data and never enter the alloc_tag maple tree. This patch returns CODETAG_MODULE_EXCLUDED when profiling is off to skip percpu counter allocation. > diff --git a/lib/codetag.c b/lib/codetag.c > index a9cda4c962a30..8506ecab9ea70 100644 > --- a/lib/codetag.c > +++ b/lib/codetag.c [ ... ] > @@ -238,9 +238,10 @@ static int codetag_module_init(struct codetag_type *cttype, struct module *mod) > } > up_write(&cttype->mod_lock); > > - if (err < 0) { > + if (err) { > + /* Error or excluded: cmod is dropped, free it. */ > kfree(cmod); > - return err; > + return err < 0 ? err : 0; > } > > return 0; [Severity: High] This is a pre-existing issue, but if codetag_module_init() fails midway through the codetag types, does it leave the module registered in the previously initialized types? lib/codetag.c:codetag_load_module() { ... list_for_each_entry(cttype, &codetag_types, link) { ret = codetag_module_init(cttype, mod); if (ret) break; } ... } If the loop breaks on an error, the successfully initialized types are not unwound. When load_module() in kernel/module/main.c handles this error, it jumps to sysfs_cleanup and eventually frees the module, but misses a call to codetag_unload_module(mod). Could this lead to a use-after-free or a memory leak when traversing the IDR later, since the freed module is still registered? [Severity: High] This is another pre-existing issue. Is the error code lost in load_module() when codetag_load_module() fails? kernel/module/main.c:load_module() { ... if (codetag_load_module(mod)) goto sysfs_cleanup; ... } Since err is not assigned before jumping to sysfs_cleanup, would the syscall incorrectly return 0 (success) even though the module failed to load and was destroyed on the error path? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=1 ^ permalink raw reply [flat|nested] 13+ 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-13 9:49 ` sashiko-bot @ 2026-08-15 6:15 ` Suren Baghdasaryan 2026-08-17 5:05 ` Hao Ge 1 sibling, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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-13 9:54 ` sashiko-bot 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, 2 replies; 13+ 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] 13+ 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-13 9:54 ` sashiko-bot 2026-08-15 6:21 ` Suren Baghdasaryan 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-08-13 9:54 UTC (permalink / raw) To: Hao Ge; +Cc: linux-modules Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] An integer overflow in ALIGN() using unvalidated sh_addralign allows an attacker to spoof the memory type stored in sh_entsize, leading to an out-of-bounds read and kernel panic. -- --- Patch [2]: [RFC PATCH 2/3] module: move codetag section placement decision to layout_sections() --- commit f621cb3d3b6809c9dd569a1f5e20bfb1e929c4d4 Author: Hao Ge <hao.ge@linux.dev> module: move codetag section placement decision to layout_sections() This commit moves the decision for codetag section placement to layout_sections(), storing the result in sh_entsize using the new MOD_MEM_CODETAG type. The move_module() function now relies on this stored type instead of calling codetag_needs_module_section() again. This prevents a race condition when profiling support is toggled during module load. > diff --git a/kernel/module/main.c b/kernel/module/main.c > index 46dd8d25a6058..25056e87d0038 100644 > --- a/kernel/module/main.c > +++ b/kernel/module/main.c [ ... ] > @@ -2815,11 +2812,10 @@ static int move_module(struct module *mod, struct load_info *info) > continue; > > sname = info->secstrings + shdr->sh_name; > - /* > - * Load codetag sections separately as they might still be used > - * after module unload. > - */ > - if (codetag_needs_module_section(mod, sname, shdr->sh_size)) { > + > + enum mod_mem_type type = shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT; > + > + if (type == MOD_MEM_CODETAG) { [Severity: High] Can a maliciously crafted ELF module spoof the type by causing an integer overflow in module_get_offset_and_type()? If a module with two SHT_NOBITS sections is loaded, it might bypass ELF file size validations. The first section could inflate mod->mem[MOD_DATA].size to 0x60000001. If the second section has sh_addralign = 0x10000000, it would cause an integer overflow in module_get_offset_and_type(): kernel/module/main.c:module_get_offset_and_type() { ... offset = ALIGN(mod->mem[type].size, sechdr->sh_addralign ?: 1); ... return offset | mask; } The overflowed offset (0x70000000) places 7 (MOD_MEM_CODETAG) into the top 4 bits of sh_entsize. By setting the second section's sh_size to 0x90000000, the attacker can wrap mod->mem[MOD_DATA].size back to 0, avoiding -ENOMEM during module_memory_alloc(). If this spoofed type is trusted by move_module(), execution routes to codetag_alloc_module_section(): lib/codetag.c:codetag_alloc_module_section() { ... const char *type_name = name + strlen(CODETAG_SECTION_PREFIX); ... } If a short-named section is routed here by the spoofed type, could blindly advancing the name pointer cause an out-of-bounds read past the secstrings vmalloc allocation and trigger a kernel panic? > dest = codetag_alloc_module_section(mod, sname, shdr->sh_size, > arch_mod_section_prepend(mod, i), shdr->sh_addralign); > if (WARN_ON(!dest)) { -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=2 ^ permalink raw reply [flat|nested] 13+ 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-13 9:54 ` sashiko-bot @ 2026-08-15 6:21 ` Suren Baghdasaryan 2026-08-17 5:37 ` Hao Ge 1 sibling, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ 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-13 9:57 ` sashiko-bot 2026-08-15 6:51 ` Suren Baghdasaryan 2 siblings, 2 replies; 13+ 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] 13+ 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-13 9:57 ` sashiko-bot 2026-08-15 6:51 ` Suren Baghdasaryan 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-08-13 9:57 UTC (permalink / raw) To: Hao Ge; +Cc: linux-modules Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Pre-existing issues: - [High] TOCTOU race in sysctl handler allows memory profiling to remain active without support structures. -- --- Patch [3]: [RFC PATCH 3/3] alloc_tag: remove /proc/allocinfo outside of mod_lock --- commit 8d4c5cdeb441b4a47f3511f205a68485820706b0 Author: Hao Ge <hao.ge@linux.dev> alloc_tag: remove /proc/allocinfo outside of mod_lock shutdown_mem_profiling() calls remove_proc_entry() from reserve_module_tags(), which runs under mod_lock held for write. remove_proc_entry() waits for readers, and a reader takes mod_lock for read in allocinfo_start(). Move remove_proc_entry() to a workqueue. > diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c > index 26c6ebc96a6ae..65d0ae9af94b2 100644 > --- a/mm/alloc_tag.c > +++ b/mm/alloc_tag.c [ ... ] > @@ -600,7 +607,7 @@ static void shutdown_mem_profiling(bool remove_file) > return; > > if (remove_file) > - remove_proc_entry(ALLOCINFO_FILE_NAME, NULL); > + schedule_work(&remove_allocinfo_work); > mem_profiling_support = false; > } [Severity: High] This isn't a bug introduced by this patch, but does shutdown_mem_profiling() have a TOCTOU race with the sysctl handler proc_mem_profiling_handler()? If a concurrent module load fails setup and calls shutdown_mem_profiling(), it disables the static key and sets mem_profiling_support = false. Concurrently, could a sysctl write to /proc/sys/vm/mem_profiling evaluate mem_profiling_support before it is set to false: mm/alloc_tag.c:proc_mem_profiling_handler() { ... if (!mem_profiling_support) return -EINVAL; } return proc_do_static_key(table, write, buffer, lenp, ppos); } and then execute proc_do_static_key() after shutdown_mem_profiling() has finished, errantly re-enabling the static key? If the static key is enabled but mem_profiling_support is false, any subsequent module load will skip allocating per-cpu tag counters. When the new module executes a memory allocation, would alloc_tag_ref_set() unconditionally dereference a NULL tag->counters? include/linux/alloc_tag.h:alloc_tag_ref_set() { ... this_cpu_inc(tag->counters->calls); ... } -- Sashiko AI review · https://sashiko.dev/#/patchset/20260813093421.135230-1-hao.ge@linux.dev?part=3 ^ permalink raw reply [flat|nested] 13+ 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-13 9:57 ` sashiko-bot @ 2026-08-15 6:51 ` Suren Baghdasaryan 2026-08-17 5:41 ` Hao Ge 1 sibling, 1 reply; 13+ 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] 13+ 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; 13+ 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] 13+ messages in thread
end of thread, other threads:[~2026-08-17 5:41 UTC | newest] Thread overview: 13+ 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-13 9:49 ` sashiko-bot 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-13 9:54 ` sashiko-bot 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-13 9:57 ` sashiko-bot 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