On 2026/8/12 14:32, Hao Ge wrote:
> CC Andrew and Suren
>
>
> On 2026/8/12 14:04, [email protected] 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 <[email protected]>
>>
>> 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.
>
>
Sorry for the messy indentation — it's down to my mail client's formatting
config.
A colleague pointed out the official recommended indent configs for kernel mail
clients,
which I'd missed entirely.
https://elixir.bootlin.com/linux/v7.2-rc7/source/Documentation/process/email-clients.rst
Sorry for all the hassle I've caused everyone earlier.
The patch fixing the per-CPU counter leak has been sent out:
https://lore.kernel.org/all/[email protected]/
Thanks
Best Regards
Hao
>
>>
>>> + }
>>> /* 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.
>>
