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

[Hao Ge]

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.

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;

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?

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?