Re: [PATCH v5 2/2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled

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

Hi Suren and Petr


On 2026/8/27 08:47, Suren Baghdasaryan wrote:
> On Wed, Aug 26, 2026 at 1:32 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>
>> On 8/16/26 5:16 PM, Suren Baghdasaryan wrote:
>>> On Sat, Aug 15, 2026 at 3:45 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>>>> On 8/12/26 7:41 AM, Hao Ge 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. On overflow, shut down
>>>>> profiling, release the reservation, and return -EAGAIN so that
>>>>> layout_and_allocate() retries with profiling disabled: codetag sections
>>>>> are then placed as regular module data and the module loads without
>>>>> profiling rather than being rejected entirely.
>>>>
>>>> When the described overflow occurs, why should codetag sections be
>>>> placed as regular module data? Will the codetag support use them in any
>>>> way, or do they simply waste space? Is the issue that alloc_hooks()
>>>> creates relocations pointing into .codetag.alloc_tags?
>>>
>>> Correct, alloc_hooks() will have references into .codetag.alloc_tags.
>>> With mem_profiling_support=false they should technically never be used
>>> but I don't think it's a good idea to skip .codetag.alloc_tags section
>>> allocation and to leave dangling pointers. Also the case described
>>> here is an outlier, so optimizing it would not yield much benefit.
>>>
>> [...]
>>>> I'm not sure this is the best approach. It's complex logic for what
>>>> appears to be an edge case related to a debugging facility. It will have
>>>> the usual problem of error paths not getting enough testing and breaking
>>>> subtly over time.
>>>>
>>>> An alternative could be to reset SHF_ALLOC on the codetag section to
>>>> remove it from further processing and have relocations that point to
>>>> this section resolve to something else. It seems that alloc_hooks_tag()
>>>> could tolerate this, since it only needs to reference the associated
>>>> alloc_tag when mem_alloc_profiling_enabled() is true and that gets
>>>> disabled by reserve_module_tags() on the overflow.
>>>
>>> Hmm, yeah if we redirect the references into .codetag.alloc_tags, that
>>> would be much better.
>>>
>>>>
>>>> It is also not an ideal approach, but I feel it could be less intrusive
>>>> to the module loader. I can put together a prototype if needed.
>>>
>>> If your approach does not cause module loading to fail when we disable
>>> profiling, then that sounds like a good idea. If it's not too much
>>> trouble, could you please send an RFC?
>>
>> The alternative approach I mentioned unfortunately doesn't work well,
>> since redirecting all relocations against .codetag.alloc_tag to
>> a different destination is nontrivial. It would require introducing
>> something like frob_relocation() that is called from each
>> architecture-specific apply_relocate()/apply_relocate_add() after the
>> addend has been decoded.
>>
>> Another option I realized is to change the order in which module
>> sections are allocated. Rather than interleaving the allocation of
>> codetag and regular sections, the module loader could first try to
>> allocate codetag sections and then allocate regular sections. If
>> allocation of a codetag section fails, it can naturally fall back to
>> being treated as a regular section. This avoids retrying the allocation
>> process, which I would prefer to avoid.
>>
>> A prototype is below.
> 
> Thanks for following up on this, Petr!

+1

> At first glance, this seems like a much cleaner approach. But it's
> also a sizable change, so it will need some testing. I'll try to run
> some test scenarios over the weekend.

I believe Petr's approach can also address the race problem pointed out by this patch:
https://lore.kernel.org/all/20260813093421.135230-3-hao.ge@linux.dev/
I will also go through this patch and run some local tests as soon as possible.

Thanks
Best Regards
Hao

> 
>>
>> --
>> Thanks,
>> Petr
>>
>>
>> diff --git a/include/linux/module.h b/include/linux/module.h
>> index 96cc98568eea..0c6f32ddcbf2 100644
>> --- a/include/linux/module.h
>> +++ b/include/linux/module.h
>> @@ -325,6 +325,8 @@ enum mod_mem_type {
>>         MOD_INIT_RODATA,
>>
>>         MOD_MEM_NUM_TYPES,
>> +
>> +       MOD_STANDALONE = -2,
>>         MOD_INVALID = -1,
>>  };
>>
>> diff --git a/kernel/module/internal.h b/kernel/module/internal.h
>> index 061161cc79d9..217bb540e361 100644
>> --- a/kernel/module/internal.h
>> +++ b/kernel/module/internal.h
>> @@ -29,6 +29,10 @@
>>  #define SH_ENTSIZE_TYPE_MASK   ((1UL << SH_ENTSIZE_TYPE_BITS) - 1)
>>  #define SH_ENTSIZE_OFFSET_MASK ((1UL << (BITS_PER_LONG - SH_ENTSIZE_TYPE_BITS)) - 1)
>>
>> +#define SH_ENTSIZE_STANDALONE                                  \
>> +       (((unsigned long)MOD_STANDALONE & SH_ENTSIZE_TYPE_MASK) \
>> +        << SH_ENTSIZE_TYPE_SHIFT)
>> +
>>  /* Maximum number of characters written by module_flags() */
>>  #define MODULE_FLAGS_BUF_SIZE (TAINT_FLAGS_COUNT + 4)
>>
>> diff --git a/kernel/module/main.c b/kernel/module/main.c
>> index d0e1e0bd2ad0..a86ae8774cd0 100644
>> --- a/kernel/module/main.c
>> +++ b/kernel/module/main.c
>> @@ -1624,7 +1624,7 @@ static int apply_relocations(struct module *mod, const struct load_info *info)
>>                  * ELF template and subsequently copy it to the per-CPU destinations.
>>                  */
>>                 if (!(info->sechdrs[infosec].sh_flags & SHF_ALLOC) &&
>> -                   (!infosec || infosec != info->index.pcpu))
>> +                   info->sechdrs[infosec].sh_entsize != SH_ENTSIZE_STANDALONE)
>>                         continue;
>>
>>                 if (info->sechdrs[i].sh_flags & SHF_RELA_LIVEPATCH)
>> @@ -1722,20 +1722,6 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
>>                         if (WARN_ON_ONCE(type == MOD_INVALID))
>>                                 continue;
>>
>> -                       /*
>> -                        * Do not allocate codetag memory as we load it into
>> -                        * preallocated contiguous memory.
>> -                        */
>> -                       if (codetag_needs_module_section(mod, sname, s->sh_size)) {
>> -                               /*
>> -                                * s->sh_entsize won't be used but populate the
>> -                                * type field to avoid confusion.
>> -                                */
>> -                               s->sh_entsize = ((unsigned long)(type) & SH_ENTSIZE_TYPE_MASK)
>> -                                               << SH_ENTSIZE_TYPE_SHIFT;
>> -                               continue;
>> -                       }
>> -
>>                         s->sh_entsize = module_get_offset_and_type(mod, type, s, i);
>>                         pr_debug("\t%s\n", sname);
>>                 }
>> @@ -1745,16 +1731,10 @@ static void __layout_sections(struct module *mod, struct load_info *info, bool i
>>  /*
>>   * Lay out the SHF_ALLOC sections in a way not dissimilar to how ld
>>   * might -- code, read-only data, read-write data, small data.  Tally
>> - * sizes, and place the offsets into sh_entsize fields: high bit means it
>> - * belongs in init.
>> + * sizes, and place the offsets into sh_entsize fields.
>>   */
>>  static void layout_sections(struct module *mod, struct load_info *info)
>>  {
>> -       unsigned int i;
>> -
>> -       for (i = 0; i < info->hdr->e_shnum; i++)
>> -               info->sechdrs[i].sh_entsize = ~0UL;
>> -
>>         pr_debug("Core section allocation order for %s:\n", mod->name);
>>         __layout_sections(mod, info, false);
>>
>> @@ -2800,7 +2780,6 @@ static int move_module(struct module *mod, struct load_info *info)
>>  {
>>         int i, ret;
>>         enum mod_mem_type t = MOD_MEM_NUM_TYPES;
>> -       bool codetag_section_found = false;
>>
>>         for_each_mod_mem_type(type) {
>>                 if (!mod->mem[type].size) {
>> @@ -2818,36 +2797,14 @@ static int move_module(struct module *mod, struct load_info *info)
>>         /* Transfer each section which specifies SHF_ALLOC */
>>         pr_debug("Final section addresses for %s:\n", mod->name);
>>         for (i = 0; i < info->hdr->e_shnum; i++) {
>> -               void *dest;
>>                 Elf_Shdr *shdr = &info->sechdrs[i];
>> -               const char *sname;
>> +               void *dest;
>>
>>                 if (!(shdr->sh_flags & SHF_ALLOC))
>>                         continue;
>>
>> -               sname = info->secstrings + shdr->sh_name;
>> -               /*
>> -                * Load codetag sections separately as they might still be used
>> -                * after module unload.
>> -                */
>> -               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 (WARN_ON(!dest)) {
>> -                               ret = -EINVAL;
>> -                               goto out_err;
>> -                       }
>> -                       if (IS_ERR(dest)) {
>> -                               ret = PTR_ERR(dest);
>> -                               goto out_err;
>> -                       }
>> -                       codetag_section_found = true;
>> -               } 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;
>> -               }
>> +               dest = mod->mem[shdr->sh_entsize >> SH_ENTSIZE_TYPE_SHIFT].base +
>> +                      (shdr->sh_entsize & SH_ENTSIZE_OFFSET_MASK);
>>
>>                 if (shdr->sh_type != SHT_NOBITS) {
>>                         /*
>> @@ -2879,8 +2836,6 @@ static int move_module(struct module *mod, struct load_info *info)
>>         module_memory_restore_rox(mod);
>>         while (t--)
>>                 module_memory_free(mod, t);
>> -       if (codetag_section_found)
>> -               codetag_free_module_sections(mod);
>>
>>         return ret;
>>  }
>> @@ -2951,9 +2906,47 @@ static bool blacklisted(const char *module_name)
>>  }
>>  core_param(module_blacklist, module_blacklist, charp, 0400);
>>
>> +/*
>> + * Allocate codetag sections separately. They are loaded into preallocated
>> + * contiguous memory because they may still be used after the module is
>> + * unloaded.
>> + *
>> + * If the separate allocation overflows and fails, allocate the section normally
>> + * so that the module can still be loaded.
>> + */
>> +static void allocate_codetag_sections(struct load_info *info)
>> +{
>> +       for (unsigned int i = 1; i < info->hdr->e_shnum; i++) {
>> +               Elf_Shdr *shdr = &info->sechdrs[i];
>> +               const char *sname = info->secstrings + shdr->sh_name;
>> +               void *dest;
>> +
>> +               if (!(shdr->sh_flags & SHF_ALLOC) ||
>> +                   !codetag_needs_module_section(info->mod, sname,
>> +                                                 shdr->sh_size))
>> +                       continue;
>> +
>> +               dest = codetag_alloc_module_section(
>> +                       info->mod, sname, shdr->sh_size,
>> +                       arch_mod_section_prepend(info->mod, i),
>> +                       shdr->sh_addralign);
>> +               if (WARN_ON(!dest) || IS_ERR(dest)) {
>> +                       /* Allocate the section as a regular section. */
>> +                       continue;
>> +               }
>> +
>> +               if (shdr->sh_type != SHT_NOBITS)
>> +                       memcpy(dest, (void *)shdr->sh_addr, shdr->sh_size);
>> +               shdr->sh_addr = (unsigned long)dest;
>> +               shdr->sh_flags &= ~(unsigned long)SHF_ALLOC;
>> +               shdr->sh_entsize = SH_ENTSIZE_STANDALONE;
>> +       }
>> +}
>> +
>>  static struct module *layout_and_allocate(struct load_info *info, int flags)
>>  {
>>         struct module *mod;
>> +       unsigned int i;
>>         int err;
>>
>>         /* Allow arches to frob section contents and sizes.  */
>> @@ -2967,9 +2960,6 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>>         if (err < 0)
>>                 return ERR_PTR(err);
>>
>> -       /* We will do a special allocation for per-cpu sections later. */
>> -       info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
>> -
>>         /*
>>          * Mark relevant sections as SHF_RO_AFTER_INIT so layout_sections() can
>>          * put them in the right place.
>> @@ -2977,18 +2967,27 @@ static struct module *layout_and_allocate(struct load_info *info, int flags)
>>          */
>>         module_mark_ro_after_init(info->hdr, info->sechdrs, info->secstrings);
>>
>> -       /*
>> -        * Determine total sizes, and put offsets in sh_entsize.  For now
>> -        * this is done generically; there doesn't appear to be any
>> -        * special cases for the architectures.
>> -        */
>> +       /* Repurpose sh_entsize to track where each section is allocated. */
>> +       for (i = 0; i < info->hdr->e_shnum; i++)
>> +               info->sechdrs[i].sh_entsize = ~0UL;
>> +
>> +       /* We will do a special allocation for per-cpu sections later. */
>> +       info->sechdrs[info->index.pcpu].sh_flags &= ~(unsigned long)SHF_ALLOC;
>> +       info->sechdrs[info->index.pcpu].sh_entsize = SH_ENTSIZE_STANDALONE;
>> +
>> +       /* Allow codetag sections to be allocated separately first. */
>> +       allocate_codetag_sections(info);
>> +
>> +       /* Determine total sizes and put offsets in sh_entsize. */
>>         layout_sections(info->mod, info);
>>         layout_symtab(info->mod, info);
>>
>>         /* Allocate and move to the final place */
>>         err = move_module(info->mod, info);
>> -       if (err)
>> +       if (err) {
>> +               codetag_free_module_sections(mod);
>>                 return ERR_PTR(err);
>> +       }
>>
>>         /* Module has been copied to its final place now: return it. */
>>         mod = (void *)info->sechdrs[info->index.mod].sh_addr;
>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c
>> index 2070e682fe10..112a014d4b89 100644
>> --- a/mm/alloc_tag.c
>> +++ b/mm/alloc_tag.c
>> @@ -950,10 +950,12 @@ 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()) {
>> +               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);
>> +                       pr_warn_once("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);
>> +                       release_module_tags(mod, false);
>> +                       return ERR_PTR(-EAGAIN);
>>                 }
>>
>>                 grow_res = vm_module_tags_populate();