Re: [PATCH v7 3/4] module: allocate codetag sections before the regular module layout

[Hao Ge <hao.ge@linux.dev>]

On 2026/9/2 16:36, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> 
> New issues:
> - [High] Per-CPU memory leak of module allocation tag counters on module unload when falling back to regular section allocation.
> - [Medium] Skipping memcpy for SHT_NOBITS codetag sections leaves them filled with uninitialized kernel memory.
> 
> Pre-existing issues:
> - [High] AB-BA deadlock between `/proc/allocinfo` file readers (`pde->in_use`) and module loading (`cttype->mod_lock`).
> --
> 
> commit 073ca6b0680c884f9d863cdcbd7d5db665a17c3b
> Author: Hao Ge <hao.ge@linux.dev>
> 
> module: allocate codetag sections before the regular module layout
> 
> Whether a codetag section goes to the codetag region is decided by
> layout_sections() and asked again in move_module(). A concurrent
> load can shut profiling down in between, and move_module() then
> copies the section to offset 0 of its regular destination,
> overwriting whatever is there.
> 
> Link: https://lore.kernel.org/all/499bb60c-c6e3-43a3-bd92-95a0567ece5e@suse.com/
> 
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index b114b815187a9..b94830c5b9a10 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
> 
> [ ... ]
> 
>> @@ -2940,6 +2901,47 @@ static bool blacklisted(const char *module_name)
>>  }
>>  core_param(module_blacklist, module_blacklist, charp, 0400);
>>  
>> +/*
>> + * Allocate codetag sections separately. They are loaded into preallocated
>> + * contiguous memory because they may still be used after the module is
>> + * unloaded.
>> + *
>> + * If the separate allocation overflows, allocate the section normally
>> + * so that the module can still be loaded.
>> + */
>> +static int allocate_codetag_sections(struct load_info *info)
>> +{
>> +	for (unsigned int i = 1; i < info->hdr->e_shnum; i++) {
>> +		Elf_Shdr *shdr = &info->sechdrs[i];
>> +		const char *sname = info->secstrings + shdr->sh_name;
>> +		void *dest;
>> +
>> +		if (!codetag_needs_module_section(info->mod, sname, shdr->sh_size))
>> +			continue;
>> +
>> +		dest = codetag_alloc_module_section(info->mod, sname, shdr->sh_size,
>> +				arch_mod_section_prepend(info->mod, i), shdr->sh_addralign);
>> +		if (WARN_ON(!dest)) {
>> +			codetag_free_module_sections(info->mod);
>> +			return -EINVAL;
>> +		}
>> +		if (dest == ERR_PTR(-EAGAIN))
>> +			/* Allocate the section as a regular section. */
>> +			continue;
> 
> [Severity: High]
> Does this code leak the per-cpu memory on module unload when falling back to
> regular section allocation?
> 
> If tags_addressable() returns false, reserve_module_tags() returns -EAGAIN
> and removes the module from mod_area_mt. The fallback mechanism here allocates
> the tags in regular module memory, but the module loading process still
> unconditionally allocates per-cpu counters for each tag, oblivious to the
> fallback or the disabled profiling state:
> 
>     for (tag = start_tag; tag < stop_tag; tag++) {
>         WARN_ON(tag->counters);
>         tag->counters = alloc_percpu(struct alloc_tag_counters);
> 
> On unload, release_module_tags() fails to find the module in mod_area_mt
> because it was removed, skipping clean_unused_counters() and leaking the
> previously allocated per-cpu memory.

This has already been fixed by the commit below:
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?id=394aff7091a0278b0495549b5d2ff3b318e639d2

> 
>> +		if (IS_ERR(dest)) {
>> +			codetag_free_module_sections(info->mod);
>> +			return PTR_ERR(dest);
>> +		}
>> +
>> +		if (shdr->sh_type != SHT_NOBITS)
>> +			memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
> 
> [Severity: Medium]
> Could this expose uninitialized kernel memory if the codetag section is
> parsed from an ELF module with the SHT_NOBITS type?
> 
> The code skips memcpy() but fails to use memset() to zero-initialize the
> destination buffer. Regular module sections are safe because
> module_memory_alloc() explicitly zeroes them, but reserve_module_tags() backs
> its memory with alloc_pages_bulk_node() without the __GFP_ZERO flag:
> 

OK. Considering that codetag is a general‑purpose framework,
it shouldn't assume anything about which codetag types exist
today or later. Will fix for the next version.


> mm/alloc_tag.c:vm_module_tags_populate() {
> ...
>     allocated = alloc_pages_bulk_node(GFP_KERNEL | __GFP_NOWARN,
>             NUMA_NO_NODE, more_pages - nr, next_page + nr);
> ...
> }
> 
>> +		shdr->sh_addr = (unsigned long)dest;
>> +		shdr->sh_entsize = SH_ENTSIZE_STANDALONE;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
> 
> [ ... ]
> 
>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
>> index 2070e682fe106..112a014d4b890 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);
> 
> [Severity: High]
> This is a pre-existing issue, but does this shutdown_mem_profiling(true) call
> introduce an AB-BA deadlock between /proc/allocinfo file readers (pde->in_use)
> and module loading (cttype->mod_lock)?
> 
> A user reading /proc/allocinfo acquires the procfs in_use reference in
> use_pde(), then blocks on down_read(&alloc_tag_cttype->mod_lock) in
> allocinfo_start().
> 
> Concurrently, if a module loads and tags_addressable() fails,
> codetag_alloc_module_section() holds down_write(&cttype->mod_lock) and calls
> shutdown_mem_profiling(true), which removes the proc entry. This removal waits
> indefinitely for the in_use reference to drop to zero, hanging both procfs
> reads and module loading.
> 
>> -			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();
> 
This has already been fixed by the commit below:
https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git/commit/?id=4cd27d57fd9468491f2ea48cee93e233c55edda8

Thanks
Best Regards
Hao