Re: [PATCH v10 2/6] alloc_tag: clean up the populate failure path

Hi Suren

Thanks for you review.

On 2026/9/16 05:09, Suren Baghdasaryan wrote:

On Mon, Sep 14, 2026 at 11:59 PM Hao Ge [off-list ref] wrote:

quoted

The reservation is already stored in the maple tree when
vm_module_tags_populate() fails. A failed load never unloads the
module, so nothing releases the entry. Release it and roll
module_tags.size back. Without the rollback a concurrent load that
already passed needs_section_mem() can reuse the freed gap, skip
vm_module_tags_populate() and write to unmapped memory.

vmap_pages_range() may have installed some PTEs before failing. A
retry to populate the same range would BUG on them, so undo them,
but only if vmap actually ran.

Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
Fixes: 0f9b685626da ("alloc_tag: populate memory for module tags as needed")

If a patch fixes two other patches, it likely needs to be split so
that each part fixes one patch only. But read on please. I'm not sure
if one of the fixes is correct.

Kept them in one patch because they only make sense together.

After a failed populate the reservation is released and the
size rolled back, so the next load takes the same gap and calls
vm_module_tags_populate() again, and that retry would trip over
the PTEs the failed vmap left behind.

Without the rollback a load that already passed needs_section_mem()
just reuses the gap, skips populate and writes to unmapped memory.
So splitting them leaves each half broken.                                   

quoted

Reported-by: Sashiko <sashiko-bot@kernel.org>
Cc: stable@vger.kernel.org
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
 mm/alloc_tag.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
index 2070e682fe10..95ddf5b743d0 100644
--- a/mm/alloc_tag.c
+++ b/mm/alloc_tag.c

@@ -804,6 +804,13 @@ static int vm_module_tags_populate(void)
                                     next_page, PAGE_SHIFT) < 0) {
                        release_pages_arg arg = { .pages = next_page };

+                       /*
+                        * vmap_pages_range() only runs once all pages were
+                        * allocated, and it may have installed some mappings
+                        * before failing. Undo them.

If vmap_pages_range() failed to map the range, why should we need to
undo it? If it indeed leaves some partial mapping when failing then I
would argue that vmap_pages_range() should be undoing these partial
mappings itself before returning the error.

Yeah it does - none of the paths under vmap_pages_range() undo themselves on error.
pcpu_map_pages does perform cleanup on its own as well. There may be other similar sites.
https://elixir.bootlin.com/linux/v7.3-rc3/source/mm/percpu-vm.c#L255
so I did the same here.

If we really want vmap_pages_range() to handle this cleanup internally, I believe that would
be a relatively large change, and we would need to consider many more cases. So I think we keep
the current behaviour for now.

Thanks
Best Regards
Hao

quoted

+                        */
+                       if (nr == more_pages)
+                               vunmap_range(phys_end, phys_end + (nr << PAGE_SHIFT));
                        /* Clean up and error out */
                        release_pages(arg, nr);
                        return -ENOMEM;

@@ -947,6 +954,7 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
                return ret;

        if (module_tags.size < offset + size) {
+               unsigned long prev_size = module_tags.size;
                int grow_res;

                module_tags.size = offset + size;

@@ -961,6 +969,8 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
                        shutdown_mem_profiling(true);
                        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);
+                       module_tags.size = prev_size;

Yes, this one is a valid fix. It fixes 0f9b685626da commit.

quoted

                        return ERR_PTR(grow_res);
                }
        }
--
2.25.1