Hi Suren and Petr
On 2026/8/27 08:47, Suren Baghdasaryan wrote: > On Wed, Aug 26, 2026 at 1:32 AM Petr Pavlu <[email protected]> wrote: >> >> On 8/16/26 5:16 PM, Suren Baghdasaryan wrote: >>> On Sat, Aug 15, 2026 at 3:45 AM Petr Pavlu <[email protected]> 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/[email protected]/ 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();
