Re: [PATCH v2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled

On Tue, Aug 4, 2026 at 7:26 PM Hao Ge <[email protected]> wrote:
>
> Hi Suren
>
>
> Thanks a lot for your review.
>
>
> On 2026/8/5 04:08, Suren Baghdasaryan wrote:
> > On Tue, Aug 4, 2026 at 5:21 AM Hao Ge <[email protected]> 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.
> >>
> >> 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. Also return an error
> >> after shutdown_mem_profiling() to skip vm_module_tags_populate(), as
> >> the mapped pages would never be reused - shutdown_mem_profiling() sets
> >> mem_profiling_support to false, so no future module load enters the
> >> codetag path.
> >>
> >> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag compression")
> >> Cc: [email protected]
> >> Signed-off-by: Hao Ge <[email protected]>
> > Thanks for the fix, Hao!
> >
> >> ---
> >> Changes in v2:
> >> - Return error after shutdown_mem_profiling() to skip unnecessary
> >>    vm_module_tags_populate()
> >>    v1 link: https://lore.kernel.org/all/[email protected]/
> >> ---
> >>   mm/alloc_tag.c | 3 ++-
> >>   1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
> >> index 52aece27b00e..d8c36430f1c0 100644
> >> --- a/mm/alloc_tag.c
> >> +++ b/mm/alloc_tag.c
> >> @@ -904,10 +904,11 @@ static void *reserve_module_tags(struct module *mod, unsigned long size,
> >>                  int grow_res;
> >>
> >>                  module_tags.size = offset + size;
> >> -               if (mem_alloc_profiling_enabled() && !tags_addressable()) {
> > Makes sense but how about replacing mem_alloc_profiling_enabled() with
> > mem_profiling_support? Otherwise this warning will be issued multiple
> > times if we are loading multiple modules.
>
>
> That's a really good point, this duplicate warning issue will indeed happen.
>
> When I thought over your point, I realized using mem_profiling_support
> creates a small race window.

Yeah, you are right.

>
>
> Thread A（insmod A）                      Thread B (insmod B)
>
> --------- ---------
>
> needs_section_mem() -> true
>
>      needs_section_mem() -> true
>
>      (profiling still supported)
>
> reserve_module_tags()
>
>      overflow -> shutdown
>
>      mem_profiling_support=false
>
>      return -ENOSPC
>
>      reserve_module_tags()
>
>          mem_profiling_support==false
>
>          -> overflow check skipped
>
>          -> vm_module_tags_populate()
>
>              maps unused tag pages
>
> So I'd rather go with pr_warn_once here.

pr_warn_once() works. Thanks!

>
>
> >> +               if (!tags_addressable()) {
> >>                          shutdown_mem_profiling(true);
> >>                          pr_warn("With module %s there are too many tags to fit in %d page flag bits. Memory allocation profiling is disabled!\n",
> >>                                  mod->name, NR_UNUSED_PAGEFLAG_BITS);
> >> +                       return ERR_PTR(-ENOSPC);
> > This ENOSPC error will be propagated all the way up to the init_module
> > syscall and it's not among the error codes currently expected (see:
> > https://man7.org/linux/man-pages/man2/init_module.2.html). I suggest
> > returning ENOMEM instead.
>
>
> Agree, will change
>
>
> Thanks
>
> Best Regards
>
> Hao
>
> >>                  }
> >>
> >>                  grow_res = vm_module_tags_populate();
> >> --
> >> 2.25.1
> >>