Re: [PATCH v11 6/7] alloc_tag: skip percpu counter allocation when profiling is disabled

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] mm: crash on freeing static percpu variable when module load fails or profiling disabled
--

commit 1ed77a1b45b5a7a434417c336e13dff08f4838d1
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.
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.

> diff --git a/lib/codetag.c b/lib/codetag.c
> index a9cda4c962a30..a0b600720afc1 100644
> --- a/lib/codetag.c
> +++ b/lib/codetag.c
[ ... ]
> @@ -388,7 +390,11 @@ void codetag_unload_module(struct module *mod)
>  			++cttype->content_id;
>  		}
>  		up_write(&cttype->mod_lock);
> -		if (found && cttype->desc.free_section_mem)
> +		/*
> +		 * A module whose module_load() returned -EOPNOTSUPP is not
> +		 * in the idr but may still hold reserved section memory.
> +		 */
> +		if (cttype->desc.free_section_mem)
>  			cttype->desc.free_section_mem(mod, true);
>  	}
>  	mutex_unlock(&codetag_lock);

[Severity: High]
Does this unconditional call to free_section_mem() lead to a crash when
freeing statically allocated percpu memory?

On architectures defining CONFIG_ARCH_MODULE_NEEDS_WEAK_PER_CPU,
DEFINE_ALLOC_TAG initializes tag->counters to the statically allocated
&_shared_alloc_tag.

If a module load skips dynamic counter allocation (either because
mem_profiling_support is false and load_module() returns -EOPNOTSUPP, or if
alloc_percpu() fails with -ENOMEM), tag->counters remains pointing to
&_shared_alloc_tag.

Because free_section_mem() is now called unconditionally here on module unload
or load failure, it triggers the cleanup path for these uninitialized tags:
release_module_tags() -> clean_unused_counters()

In clean_unused_counters(), since the static _shared_alloc_tag has 0 bytes,
it unconditionally calls free_percpu(tag->counters):

mm/alloc_tag.c:clean_unused_counters() {
	...
		if (!counter.bytes) {
			free_percpu(tag->counters);
			tag->counters = NULL;
	...
}

Calling free_percpu() on a statically allocated per-cpu variable that doesn't
belong to a managed chunk can corrupt memory and cause a crash.

> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> index 1a6108a596684..1106fc60dc89c 100644
> --- a/mm/alloc_tag.c
> +++ b/mm/alloc_tag.c
> @@ -981,6 +981,10 @@ 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 without its tags. */
> +	if (!mem_profiling_support)
> +		return -EOPNOTSUPP;
> +
>  	/* percpu counters for core allocations are already statically allocated */
>  	if (!mod)
>  		return 0;

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