mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Hao Ge <hao.ge@linux.dev>
To: Suren Baghdasaryan <surenb@google.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	Abhishek Bapat <abhishekbapat@google.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH v2] alloc_tag: fix undetected compressed tag overflow when profiling is disabled
Date: Wed, 5 Aug 2026 10:51:39 +0800	[thread overview]
Message-ID: <b2684958-5c39-4a9f-ab83-2e665406c0c0@linux.dev> (raw)
In-Reply-To: <bd95f8c6-33e2-4fda-bd4b-ebc37ee7c0fb@linux.dev>


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 <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.
>>>
>>> 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: stable@vger.kernel.org
>>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> 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/20260804064408.105033-1-hao.ge@linux.dev/
>>> ---
>>>   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
>>>

  reply	other threads:[~2026-08-05  2:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-04 12:20 Hao Ge
2026-08-04 19:43 ` Andrew Morton
2026-08-05  8:35   ` Hao Ge
2026-08-04 20:08 ` Suren Baghdasaryan
2026-08-05  2:26   ` Hao Ge
2026-08-05  2:51     ` Hao Ge [this message]
2026-08-05 14:46     ` Suren Baghdasaryan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b2684958-5c39-4a9f-ab83-2e665406c0c0@linux.dev \
    --to=hao.ge@linux.dev \
    --cc=abhishekbapat@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=stable@vger.kernel.org \
    --cc=surenb@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®