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

[Suren Baghdasaryan]

On Thu, Aug 6, 2026 at 1:36 AM Hao Ge <hao.ge@xxxxxxxxx> wrote:
>
> Hi Suren
>
>
> On 2026/8/6 01:47, Suren Baghdasaryan wrote:
> > On Wed, Aug 5, 2026 at 9:55 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >>
> >> On Wed,  5 Aug 2026 17:06:33 +0800 Hao Ge <hao.ge@xxxxxxxxx> 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. 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.
> >>
> >> Thanks.
> >>
> >> AI review points at a cpuple of possible things, one pre-existing:
> >>          https://sashiko.dev/#/patchset/20260805090633.141001-1-hao.ge@xxxxxxxxx
> >
> > Yes, pre-existing issue can be handled separately, it's not directly
> > related to this change.
> >
> >>
> >> "Does this unintentionally result in a denial of service for module
> >> loading, preventing critical drivers from loading when they otherwise
> >> could have just disabled profiling and gracefully continued?" sounds
> >> pretty obscure and I doubt if we care?
> >
> > Hmm, I think Hao considered that in his previous version
>
> Yes, I did look into this.
>
> but now I'm
> > thinking we might be able to handle this more gracefully and not fail
> > the module loading. When profiling is disabled
> > codetag_needs_module_section() returns false here:
> > https://elixir.bootlin.com/linux/v7.1.5/source/kernel/module/main.c#L2822,
> > so if we had to disable profiling, we could return NULL instead of
> > ENOMEM and change the condition here:
> > https://elixir.bootlin.com/linux/v7.1.5/source/kernel/module/main.c#L2825
> > to act as if codetag_needs_module_section() was false from the
> > beginning. IOW code at
> > https://elixir.bootlin.com/linux/v7.1.5/source/kernel/module/main.c#L2822
> > becomes:
> >
> > if (codetag_needs_module_section(mod, sname, shdr->sh_size) &&
> >      (dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
> >        arch_mod_section_prepend(mod, i), shdr->sh_addralign)) != NULL) {
> > ...
>
> The primary blocker is __layout_sections.
>
> https://elixir.bootlin.com/linux/v7.1.5/source/kernel/module/main.c#L1730
>
> This is because __layout_sections controls whether we reserve space for
> the codetag section inside EXECMEM_MODULE_DATA, the same way we handle
> regular sections.
>
> When codetag_needs_module_section() returns true during layout,
> module_get_offset_and_type() is skipped, so sh_entsize only gets the
> type bits with offset = 0 - no space is reserved.
>
> If we bail out with NULL when compressed tags overflow and add a dest !=
> NULL check here, we'll end up hitting the later else block.
>
> if (codetag_needs_module_section(mod, sname, shdr->sh_size) &&
>      (dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
> arch_mod_section_prepend(mod, i), shdr->sh_addralign)) != NULL) {
>         ......
> } else {
>         enum mod_mem_type type = shdr->sh_entsize  >> SH_ENTSIZE_TYPE_SHIFT;
>         unsigned long offset = shdr->sh_entsize  & SH_ENTSIZE_OFFSET_MASK;
>         dest = mod->mem[type].base + offset;
> }
>
> That'll cause the following memcpy to clobber the leading data.

Ah, ok, now I remember how the areas are reserved there. Yes, my
suggestion would not work.

>
> If we really need to fix this, we can use dest's return value, with a
> check like:
> if (codetag_needs_module_section(mod, sname, shdr->sh_size)) {
>         dest = codetag_alloc_module_section(mod, sname, shdr->sh_size,
>                 arch_mod_section_prepend(mod, i), shdr->sh_addralign);
>         if ( dest == sentinel_val )
>                 continue;

You can't simply continue here because during the next iteration
codetag_needs_module_section() will return false (since we called
shutdown_mem_profiling() and set mem_profiling_support to false) and
will take the "else" branch you pointed out earlier, which will
clobber the leading data.

>
> But that would also mean extra work on our end - for instance, we’d need
> to stop the per-cpu allocations inside codetag_load_module.
>
> https://elixir.bootlin.com/linux/v7.1.5/source/kernel/module/main.c#L3571
>
> Could there be a simpler solution for this?

Hmm. What if we return something like -EAGAIN and propagate it up to
load_module(). load_module() will check for that error and retry
calling layout_and_allocate() but since now
mem_profiling_support=false, both layout_sections() and move_module()
will work as if profiling is disabled. We need to make sure
codetag_module_replaced() and codetag_load_module() do nothing when
mem_profiling_support=false but that should be easy. WDYT?

>
> Thanks
> Best Regards
>
> Hao
>
> >
> > I think this would result in a better handling of this situation: if
> > we can't fit the tags anymore, we issue a warning, disable profiling
> > but continue loading the module. Hao, WDYT?
>
>
>
>
>
>