Re: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled

On Sat, Aug 15, 2026 at 3:45 AM Petr Pavlu <[email protected]> wrote:
>
> On 8/12/26 7:41 AM, Hao Ge wrote:
> > In reserve_module_tags(), the tag overflow check is gated on
> > mem_alloc_profiling_enabled():
> >
> >     if (mem_alloc_profiling_enabled() && !tags_addressable())
> >
> > If profiling is toggled off at runtime and a module is loaded whose
> > tags exceed the compressed-mode limit, shutdown_mem_profiling() is
> > skipped. vm_module_tags_populate() still maps memory for the tags and
> > the module loads successfully, but the total tag count now exceeds what
> > NR_UNUSED_PAGEFLAG_BITS can address.
> >
> > Once profiling is re-enabled, ref_to_idx() computes each tag's index
> > as its position in the alloc_tag array. update_page_tag_ref() masks
> > it to alloc_tag_ref_mask before storing in page->flags. Indices
> > beyond the mask are truncated and idx_to_ref() resolves them to wrong
> > tags.
> >
> > This silently corrupts /proc/allocinfo: allocated pages get attributed
> > to the wrong call sites, so the statistics it reports are wrong.
> >
> > mem_alloc_profiling_enabled() and mem_profiling_compressed are
> > independent. Once compressed mode is established at boot, it stays
> > active regardless of runtime toggles of mem_profiling.
> >
> > Remove the mem_alloc_profiling_enabled() guard. On overflow, shut down
> > profiling, release the reservation, and return -EAGAIN so that
> > layout_and_allocate() retries with profiling disabled: codetag sections
> > are then placed as regular module data and the module loads without
> > profiling rather than being rejected entirely.
>
> When the described overflow occurs, why should codetag sections be
> placed as regular module data? Will the codetag support use them in any
> way, or do they simply waste space? Is the issue that alloc_hooks()
> creates relocations pointing into .codetag.alloc_tags?

Correct, alloc_hooks() will have references into .codetag.alloc_tags.
With mem_profiling_support=false they should technically never be used
but I don't think it's a good idea to skip .codetag.alloc_tags section
allocation and to leave dangling pointers. Also the case described
here is an outlier, so optimizing it would not yield much benefit.

>
> >
> > Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> > Cc: [email protected]
> > Suggested-by: Suren Baghdasaryan <[email protected]>
> > Signed-off-by: Hao Ge <[email protected]>
> > ---
> >  kernel/module/main.c | 25 +++++++++++++++++++++++--
> >  mm/alloc_tag.c       |  8 +++++---
> >  2 files changed, 28 insertions(+), 5 deletions(-)
> >
> > diff --git a/kernel/module/main.c b/kernel/module/main.c
> > index 46dd8d25a605..ed26f167be84 100644
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -2944,6 +2944,7 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
> >  {
> >       struct module *mod;
> >       int err;
> > +     unsigned long frob_size[MOD_MEM_NUM_TYPES];
>
> frob_size is used to store values of module_memory::size, which has type
> `unsigned int`. The types should match.
>
> >
> >       /* Allow arches to frob section contents and sizes.  */
> >       err = module_frob_arch_sections(info->hdr, info->sechdrs,
> > @@ -2966,18 +2967,38 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
> >        */
> >       module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
> >
> > +     /*
> > +      * Save the sizes reserved by module_frob_arch_sections() so they can
> > +      * be restored if we retry below.
> > +      */
> > +     for_each_mod_mem_type(type)
> > +             frob_size[type] = info->mod->mem[type].size;
> > +
> >       /*
> >        * Determine total sizes, and put offsets in sh_entsize.  For now
> >        * this is done generically; there doesn't appear to be any
> >        * special cases for the architectures.
> >        */
> > +retry:
> >       layout_sections(info->mod, info);
> >       layout_symtab(info->mod, info);
> >
> >       /* Allocate and move to the final place */
> >       err = move_module(info->mod, info);
> > -     if (err)
> > -             return ERR_PTR(err);
> > +     if (err) {
> > +             if (err != -EAGAIN)
> > +                     return ERR_PTR(err);
>
> The move_module() logic is non-trivial. -EAGAIN could be returned by
> other code, now or in the future.

That's a good point.

>
> > +             /*
> > +              * -EAGAIN means profiling was disabled but the module
> > +              * can still load without it. Reset state and retry.
> > +              */
> > +             rewrite_section_headers(info, flags);
> > +             for_each_mod_mem_type(type)
> > +                     info->mod->mem[type].size = frob_size[type];
> > +             info->sechdrs[info->index.sym].sh_flags &= ~(unsigned long)SHF_ALLOC;
> > +             info->sechdrs[info->index.str].sh_flags &= ~(unsigned long)SHF_ALLOC;
>
> Why is it necessary to reset SHF_ALLOC for .symtab and .strtab here?

I believe layout_symtab() sets that bit and to retry we need to reset
it. But I might be wrong here.

>
> > +             goto retry;
> > +     }
> >
> >       /* Module has been copied to its final place now: return it. */
> >       mod = (void *)info->sechdrs[info->index.mod].sh_addr;
>
> I'm not sure this is the best approach. It's complex logic for what
> appears to be an edge case related to a debugging facility. It will have
> the usual problem of error paths not getting enough testing and breaking
> subtly over time.
>
> An alternative could be to reset SHF_ALLOC on the codetag section to
> remove it from further processing and have relocations that point to
> this section resolve to something else. It seems that alloc_hooks_tag()
> could tolerate this, since it only needs to reference the associated
> alloc_tag when mem_alloc_profiling_enabled() is true and that gets
> disabled by reserve_module_tags() on the overflow.

Hmm, yeah if we redirect the references into .codetag.alloc_tags, that
would be much better.

>
> It is also not an ideal approach, but I feel it could be less intrusive
> to the module loader. I can put together a prototype if needed.

If your approach does not cause module loading to fail when we disable
profiling, then that sounds like a good idea. If it's not too much
trouble, could you please send an RFC?

>
> --
> Thanks,
> Petr