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

Hi Suren


On 2026/8/6 01:47, Suren Baghdasaryan wrote:
> On Wed, Aug 5, 2026 at 9:55 AM Andrew Morton <[email protected]> wrote:
>>
>> On Wed,  5 Aug 2026 17:06:33 +0800 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.
>>>
>>> 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/[email protected]
> 
> 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?