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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Percpu counters are permanently leaked when module alloc_tags fall back to regular module memory.
--

commit e3319858bb95e951205b0e09569d4ef2ec702635
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/ [1]

> diff --git a/kernel/module/main.c b/kernel/module/main.c
> index 15ad6c54bb1c3..f020293b3a496 100644
> --- a/kernel/module/main.c
> +++ b/kernel/module/main.c

[ ... ]

> @@ -2947,6 +2908,49 @@ 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 error fallback path lead to a permanent percpu memory leak?

When the dedicated tag memory area is full and tags_addressable() returns
false, reserve_module_tags() returns -EAGAIN. This code bypasses the error via
continue, falling back to allocating the .alloc_tags section as a regular
module data section instead of failing the module load.

However, mm/alloc_tag.c:load_module() still unconditionally allocates percpu
counters for these tags:

    tag->counters = alloc_percpu(struct alloc_tag_counters);

When the module later unloads, mm/alloc_tag.c:release_module_tags() attempts
to clean up:

    if (!val) /* module not found */
        goto out;

Since the module's tags fell back to regular memory, the module is not found
in the mod_area_mt maple tree, and release_module_tags() immediately returns.
This skips clean_unused_counters() and appears to permanently leak the percpu
counters for every tag in the module.

Could this eventual exhaustion of percpu memory be avoided by either skipping
the percpu allocation or ensuring the cleanup logic can find modules that fell
back to regular memory?

> +		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);
> +		else
> +			memset(dest, 0, shdr->sh_size);
> +		shdr->sh_addr = (unsigned long)dest;
> +		shdr->sh_entsize = SH_ENTSIZE_STANDALONE;
> +	}
> +
> +	return 0;
> +}
> +

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