Re: [PATCH v2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
Hao Ge <[email protected]> Wed, 5 Aug 2026 10:51:39 +0800
| Newsgroups | org.kvack.linux-mm,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
On 2026/8/5 10:26, Hao Ge wrote: > Hi Suren > > > Thanks a lot for your review. > > > On 2026/8/5 04:08, Suren Baghdasaryan wrote: >> On Tue, Aug 4, 2026 at 5:21 AM 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. >>> >>> 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. >>> >>> Fixes: 4835f747d3ed ("alloc_tag: support for page allocation tag >>> compression") >>> Cc: [email protected] >>> Signed-off-by: Hao Ge <[email protected]> >> Thanks for the fix, Hao! >> >>> --- >>> Changes in v2: >>> - Return error after shutdown_mem_profiling() to skip unnecessary >>> vm_module_tags_populate() >>> v1 link: >>> https://lore.kernel.org/all/[email protected]/ >>> --- >>> mm/alloc_tag.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/mm/alloc_tag.c b/mm/alloc_tag.c >>> index 52aece27b00e..d8c36430f1c0 100644 >>> --- a/mm/alloc_tag.c >>> +++ b/mm/alloc_tag.c >>> @@ -904,10 +904,11 @@ 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()) { >> Makes sense but how about replacing mem_alloc_profiling_enabled() with >> mem_profiling_support? Otherwise this warning will be issued multiple >> times if we are loading multiple modules. > > > That's a really good point, this duplicate warning issue will indeed > happen. > > When I thought over your point, I realized using mem_profiling_support > creates a small race window. > > > Thread A(insmod A) Thread B (insmod B) > > --------- --------- > > needs_section_mem() -> true > > needs_section_mem() -> true > > (profiling still supported) > > reserve_module_tags() > > overflow -> shutdown > > mem_profiling_support=false > > return -ENOSPC > > reserve_module_tags() > > mem_profiling_support==false > > -> overflow check skipped > > -> vm_module_tags_populate() > > maps unused tag pages Sorry, my mail client messed up the race diagram, resend it. Thread A (insmod A) Thread B (insmod B) --------------------- --------------------- needs_section_mem() -> true needs_section_mem() -> true (profiling still supported) reserve_module_tags() overflow -> shutdown_mem_profiling() mem_profiling_support = false return -ENOMEM reserve_module_tags() mem_profiling_support == false -> overflow check skipped -> vm_module_tags_populate() maps unused tag pages > > So I'd rather go with pr_warn_once here. > > >>> + 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); >>> + return ERR_PTR(-ENOSPC); >> This ENOSPC error will be propagated all the way up to the init_module >> syscall and it's not among the error codes currently expected (see: >> https://man7.org/linux/man-pages/man2/init_module.2.html). I suggest >> returning ENOMEM instead. > > > Agree, will change > > > Thanks > > Best Regards > > Hao > >>> } >>> >>> grow_res = vm_module_tags_populate(); >>> -- >>> 2.25.1 >>>