Re: [PATCH v8 4/4] alloc_tag: release the reservation when populate fails

On 2026/9/7 14:43, sashiko-bot@kernel.org wrote:

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] The patch introduces a kernel crash by freeing the maple tree entry without rolling back module_tags.size on failure.

Pre-existing issues:
- [High] vmap_pages_range() failure in vm_module_tags_populate() leaves physical pages mapped while freeing them, leading to Use-After-Free.
- [High] AB-BA Deadlock between procfs read and module loading via shutdown_mem_profiling().
- [Medium] Memory leak of percpu counters when module falls back to standard section allocation.
--

commit 09d0b9820f6ab861e4c9680b44a685c8658fb571
Author: Hao Ge [off-list ref]

alloc_tag: release the reservation when populate fails

vm_module_tags_populate() can fail after a reservation is stored in the
maple tree. The error return previously leaked the entry because a failed
load never unloads the module. This patch calls release_module_tags()
on failure to free the reservation.

quoted

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 112a014d4b890..c11813cd9e153 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c

@@ -963,6 +963,7 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
 			shutdown_mem_profiling(true);

[Severity: High]
This isn't a bug introduced by this patch, but could calling
shutdown_mem_profiling(true) here risk an AB-BA deadlock?

A read to /proc/allocinfo enters proc_reg_read_iter(), taking an 'in_use'
reference on the proc entry, then calls allocinfo_start() which waits for
down_read(&alloc_tag_cttype->mod_lock).

Concurrently, module loading calls codetag_alloc_module_section(), holding
down_write(&alloc_tag_cttype->mod_lock).

If reserve_module_tags() encounters an error, it calls
shutdown_mem_profiling(true), which invokes remove_proc_entry().

Since remove_proc_entry() blocks waiting for the 'in_use' reference count
to drop to zero, and the procfs reader blocks on the mod_lock held by the
writer, doesn't this create a system hang during a module load failure?

quoted

 			pr_err("Failed to allocate memory for allocation tags in the module %s. Memory allocation profiling is disabled!\n",
 			       mod->name);
+			release_module_tags(mod, false);

[Severity: Critical]
Will this new call to release_module_tags() cause a kernel crash on a
subsequent module load?

In reserve_module_tags(), module_tags.size is optimistically updated
(module_tags.size = offset + size;) before attempting to allocate physical
memory via vm_module_tags_populate().

If population fails, this patch frees the maple tree entry, but
module_tags.size is left artificially inflated.

A subsequent module load will search the maple tree and reuse this freed gap.
Because module_tags.size is already inflated, the allocation check bypasses
vm_module_tags_populate(), returning an unmapped virtual address. Can copying
section data into this address then trigger a fatal page fault?

I see.

No, not for a subsequent load. The failure path shuts profiling
down before returning:

grow_res = vm_module_tags_populate();
if (grow_res) {
	shutdown_mem_profiling(true); /* mem_profiling_support = false */
	...
	release_module_tags(mod, false);
	 return ERR_PTR(grow_res);
}

and any later load bails out in needs_section_mem() long before it
reaches reserve_module_tags():
	if (!mem_profiling_support)
		return false;          

so no new reservation is made and the unmapped address is never
handed out.

But this reminds me of another race condition:

CPU0: failing load                 	CPU1: concurrent load                                                                           
----------------------             	---------------------------
needs_section_mem(): true         	needs_section_mem(): true
reserve_module_tags(): reserve the gap,
	module_tags.size = end
	populate() fails, gap released
                                  	reserve_module_tags(): reuse the gap,
                                  	end <= size -> skip populate
                                  	memcpy() -> page fault

I have a fix for this, but I'd like to wait for Petr and Suren to review the
remaining parts before sending it out.

Thanks
Best Regards
Hao

[Severity: Medium]
This is a pre-existing issue, but might the use of release_module_tags()
when falling back to a standard section allocation leak percpu counters?

If tags are not addressable, reserve_module_tags() calls
release_module_tags(mod, false) to remove the module from the maple tree
and returns -EAGAIN.

The module loader catches -EAGAIN and falls back to a normal section
allocation. Later, load_module() successfully allocates percpu counters for
the tags.

On unload, release_module_tags(mod, true) is called to free the counters.
However, because the module is no longer in the maple tree, the search fails
and exits early:

mm/alloc_tag.c:release_module_tags() {
    ...
    if (!val) /* module not found */
        goto out;
    ...
}

Are the percpu counters allocated during load_module() permanently leaked
in this scenario?

quoted

 			return ERR_PTR(grow_res);

[Severity: High]
This is a pre-existing issue, but is there a risk that the failure path of
vm_module_tags_populate() leaves dangling PTEs causing a Use-After-Free?

In vm_module_tags_populate(), vmap_pages_range() maps allocated pages into
the kernel virtual address space.

If it fails, it returns an error but may leave successfully mapped PTEs
active in the page tables. The error path calls release_pages() to free the
physical pages back to the allocator, but fails to call vunmap_range():

mm/alloc_tag.c:vm_module_tags_populate() {
    ...
    if (nr < more_pages || vmap_pages_range(...) < 0) {
        release_pages(arg, nr);
        return -ENOMEM;
    }
    ...
}

Could this leave dangling PTEs with PAGE_KERNEL permissions pointing to
freed pages, risking data corruption when the pages are reallocated?

quoted

 		}
 	}