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

[Hao Ge <hao.ge@linux.dev>]

Hi Suren


On 2026/8/7 09:08, Suren Baghdasaryan wrote:
> On Thu, Aug 6, 2026 at 1:36 AM Hao Ge <hao.ge@linux.dev> wrote:
>>
>> Hi Suren
>>
>>
>> On 2026/8/6 01:47, Suren Baghdasaryan wrote:
>>> On Wed, Aug 5, 2026 at 9:55 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>>>>
>>>> On Wed,  5 Aug 2026 17:06:33 +0800 Hao Ge <hao.ge@linux.dev> 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@linux.dev
>>>
>>> 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.
> 

Ah, right.

>>
>> 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?
> 

Good point, I'll look into this approach.

Thanks
Best Regards
Hao

>>
>> 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?
>>
>>
>>
>>
>>
>>