Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload
@ 2026-06-04  6:59 Hao Ge
  2026-06-04 19:24 ` Suren Baghdasaryan
  0 siblings, 1 reply; 3+ messages in thread
From: Hao Ge @ 2026-06-04  6:59 UTC (permalink / raw)
  To: Suren Baghdasaryan, Kent Overstreet, Andrew Morton
  Cc: linux-kernel, linux-mm, Hao Ge

allocinfo_start() only reinitializes the codetag iterator at position 0.
For subsequent reads (position > 0), it reuses cached iterator state from
the previous batch.  allocinfo_stop() drops mod_lock between read batches,
which allows module unload to complete and free the module memory that the
cached iterator still references:

  CPU0 (read)                        CPU1 (rmmod)
  ----                               ----
  allocinfo_start(pos=0)
    down_read(mod_lock)
    allocinfo_show()
    ...
  allocinfo_stop()
    up_read(mod_lock)
                                     codetag_unload_module()
                                       kfree(cmod)
                                       release_module_tags()
                                     ...
                                     free_mod_mem()
  allocinfo_start(pos=N)
    down_read(mod_lock)
    // reuses cached iter, skips re-init
  allocinfo_show()
    ct->filename   <-- UAF

After free_mod_mem() frees the module's .rodata, allocinfo_show()
dereferences ct->filename, ct->function which point there.

Save the iterator state in allocinfo_next() and resume from it in
allocinfo_start() with codetag_next_ct(), which detects module removal
via idr_find() returning NULL and skips to the next module.

Fixes: 9f44df50fee4 ("alloc_tag: keep codetag iterator active between read()")
Suggested-by: Suren Baghdasaryan <surenb@google.com>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
v2:
- save the iterator state in allocinfo_next() and resume from it in
  allocinfo_start() with codetag_next_ct(), which detects module removal
  via idr_find() returning NULL and skips to the next module (Suggested
  by Suren).
  v1 link: https://lore.kernel.org/all/20260525072117.112779-1-hao.ge@linux.dev/
---
 lib/alloc_tag.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
index f2f574bcf383..551cc14bb1fd 100644
--- a/lib/alloc_tag.c
+++ b/lib/alloc_tag.c
@@ -45,6 +45,7 @@ int alloc_tag_ref_offs;
 
 struct allocinfo_private {
 	struct codetag_iterator iter;
+	struct codetag_iterator reported_iter;
 	bool print_header;
 };
 
@@ -58,16 +59,20 @@ static void *allocinfo_start(struct seq_file *m, loff_t *pos)
 	if (node == 0) {
 		priv->print_header = true;
 		priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
-		codetag_next_ct(&priv->iter);
+	} else {
+		priv->iter = priv->reported_iter;
 	}
+	codetag_next_ct(&priv->iter);
 	return priv->iter.ct ? priv : NULL;
 }
 
 static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
 {
 	struct allocinfo_private *priv = (struct allocinfo_private *)arg;
-	struct codetag *ct = codetag_next_ct(&priv->iter);
+	struct codetag *ct;
 
+	priv->reported_iter = priv->iter;
+	ct = codetag_next_ct(&priv->iter);
 	(*pos)++;
 	if (!ct)
 		return NULL;
-- 
2.25.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload
  2026-06-04  6:59 [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload Hao Ge
@ 2026-06-04 19:24 ` Suren Baghdasaryan
  2026-06-05  7:43   ` Hao Ge
  0 siblings, 1 reply; 3+ messages in thread
From: Suren Baghdasaryan @ 2026-06-04 19:24 UTC (permalink / raw)
  To: Hao Ge; +Cc: Kent Overstreet, Andrew Morton, linux-kernel, linux-mm

On Thu, Jun 4, 2026 at 12:00 AM Hao Ge <hao.ge@linux.dev> wrote:
>
> allocinfo_start() only reinitializes the codetag iterator at position 0.
> For subsequent reads (position > 0), it reuses cached iterator state from
> the previous batch.  allocinfo_stop() drops mod_lock between read batches,
> which allows module unload to complete and free the module memory that the
> cached iterator still references:
>
>   CPU0 (read)                        CPU1 (rmmod)
>   ----                               ----
>   allocinfo_start(pos=0)
>     down_read(mod_lock)
>     allocinfo_show()
>     ...
>   allocinfo_stop()
>     up_read(mod_lock)
>                                      codetag_unload_module()
>                                        kfree(cmod)
>                                        release_module_tags()
>                                      ...
>                                      free_mod_mem()
>   allocinfo_start(pos=N)
>     down_read(mod_lock)
>     // reuses cached iter, skips re-init
>   allocinfo_show()
>     ct->filename   <-- UAF
>
> After free_mod_mem() frees the module's .rodata, allocinfo_show()
> dereferences ct->filename, ct->function which point there.
>
> Save the iterator state in allocinfo_next() and resume from it in
> allocinfo_start() with codetag_next_ct(), which detects module removal
> via idr_find() returning NULL and skips to the next module.
>
> Fixes: 9f44df50fee4 ("alloc_tag: keep codetag iterator active between read()")
> Suggested-by: Suren Baghdasaryan <surenb@google.com>
> Signed-off-by: Hao Ge <hao.ge@linux.dev>

Acked-by: Suren Baghdasaryan <surenb@google.com>

Thanks!

> ---
> v2:
> - save the iterator state in allocinfo_next() and resume from it in
>   allocinfo_start() with codetag_next_ct(), which detects module removal
>   via idr_find() returning NULL and skips to the next module (Suggested
>   by Suren).
>   v1 link: https://lore.kernel.org/all/20260525072117.112779-1-hao.ge@linux.dev/
> ---
>  lib/alloc_tag.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
> index f2f574bcf383..551cc14bb1fd 100644
> --- a/lib/alloc_tag.c
> +++ b/lib/alloc_tag.c
> @@ -45,6 +45,7 @@ int alloc_tag_ref_offs;
>
>  struct allocinfo_private {
>         struct codetag_iterator iter;
> +       struct codetag_iterator reported_iter;

nit: I would call it maybe prev_iter since it's not yet reported (you
store it in the allocinfo_next(), not in allocinfo_show()) but that's
a minor detail.

>         bool print_header;
>  };
>
> @@ -58,16 +59,20 @@ static void *allocinfo_start(struct seq_file *m, loff_t *pos)
>         if (node == 0) {
>                 priv->print_header = true;
>                 priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
> -               codetag_next_ct(&priv->iter);
> +       } else {
> +               priv->iter = priv->reported_iter;
>         }
> +       codetag_next_ct(&priv->iter);
>         return priv->iter.ct ? priv : NULL;
>  }
>
>  static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
>  {
>         struct allocinfo_private *priv = (struct allocinfo_private *)arg;
> -       struct codetag *ct = codetag_next_ct(&priv->iter);
> +       struct codetag *ct;
>
> +       priv->reported_iter = priv->iter;
> +       ct = codetag_next_ct(&priv->iter);
>         (*pos)++;
>         if (!ct)
>                 return NULL;
> --
> 2.25.1
>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload
  2026-06-04 19:24 ` Suren Baghdasaryan
@ 2026-06-05  7:43   ` Hao Ge
  0 siblings, 0 replies; 3+ messages in thread
From: Hao Ge @ 2026-06-05  7:43 UTC (permalink / raw)
  To: Suren Baghdasaryan; +Cc: Kent Overstreet, Andrew Morton, linux-kernel, linux-mm

Hi Suren


On 2026/6/5 03:24, Suren Baghdasaryan wrote:
> On Thu, Jun 4, 2026 at 12:00 AM Hao Ge <hao.ge@linux.dev> wrote:
>> allocinfo_start() only reinitializes the codetag iterator at position 0.
>> For subsequent reads (position > 0), it reuses cached iterator state from
>> the previous batch.  allocinfo_stop() drops mod_lock between read batches,
>> which allows module unload to complete and free the module memory that the
>> cached iterator still references:
>>
>>    CPU0 (read)                        CPU1 (rmmod)
>>    ----                               ----
>>    allocinfo_start(pos=0)
>>      down_read(mod_lock)
>>      allocinfo_show()
>>      ...
>>    allocinfo_stop()
>>      up_read(mod_lock)
>>                                       codetag_unload_module()
>>                                         kfree(cmod)
>>                                         release_module_tags()
>>                                       ...
>>                                       free_mod_mem()
>>    allocinfo_start(pos=N)
>>      down_read(mod_lock)
>>      // reuses cached iter, skips re-init
>>    allocinfo_show()
>>      ct->filename   <-- UAF
>>
>> After free_mod_mem() frees the module's .rodata, allocinfo_show()
>> dereferences ct->filename, ct->function which point there.
>>
>> Save the iterator state in allocinfo_next() and resume from it in
>> allocinfo_start() with codetag_next_ct(), which detects module removal
>> via idr_find() returning NULL and skips to the next module.
>>
>> Fixes: 9f44df50fee4 ("alloc_tag: keep codetag iterator active between read()")
>> Suggested-by: Suren Baghdasaryan <surenb@google.com>
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
> Acked-by: Suren Baghdasaryan <surenb@google.com>
>
> Thanks!
>
>> ---
>> v2:
>> - save the iterator state in allocinfo_next() and resume from it in
>>    allocinfo_start() with codetag_next_ct(), which detects module removal
>>    via idr_find() returning NULL and skips to the next module (Suggested
>>    by Suren).
>>    v1 link: https://lore.kernel.org/all/20260525072117.112779-1-hao.ge@linux.dev/
>> ---
>>   lib/alloc_tag.c | 9 +++++++--
>>   1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>> index f2f574bcf383..551cc14bb1fd 100644
>> --- a/lib/alloc_tag.c
>> +++ b/lib/alloc_tag.c
>> @@ -45,6 +45,7 @@ int alloc_tag_ref_offs;
>>
>>   struct allocinfo_private {
>>          struct codetag_iterator iter;
>> +       struct codetag_iterator reported_iter;
> nit: I would call it maybe prev_iter since it's not yet reported (you
> store it in the allocinfo_next(), not in allocinfo_show()) but that's
> a minor detail.
I see...
>>          bool print_header;
>>   };
>>
>> @@ -58,16 +59,20 @@ static void *allocinfo_start(struct seq_file *m, loff_t *pos)
>>          if (node == 0) {
>>                  priv->print_header = true;
>>                  priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
>> -               codetag_next_ct(&priv->iter);
>> +       } else {
>> +               priv->iter = priv->reported_iter;
>>          }
>> +       codetag_next_ct(&priv->iter);
>>          return priv->iter.ct ? priv : NULL;
>>   }
>>
>>   static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
>>   {
>>          struct allocinfo_private *priv = (struct allocinfo_private *)arg;
>> -       struct codetag *ct = codetag_next_ct(&priv->iter);
>> +       struct codetag *ct;
>>
>> +       priv->reported_iter = priv->iter;
>> +       ct = codetag_next_ct(&priv->iter);

AFAIK, in the seq_file call flow (start -> show -> next -> show -> next 
-> ...):

start(0): iter = get_ct_iter(); next_ct() -> iter.ct = A

show():   dump entry with iter.ct = A

next():   reported_iter = iter(A); next_ct() -> iter.ct = B

show():   dump entry with iter.ct = B

next():   reported_iter = iter(B); next_ct() -> iter.ct = C

show():   dump entry with iter.ct = C

The same holds for the overflow case in the Fill loop:

next():   reported_iter = iter(C); next_ct() -> iter.ct = D

show():   overflow, output rolled back, break

stop()

on the next read:

start():  iter = reported_iter(C); next_ct() -> iter.ct = D

show():   dump entry with iter.ct = D

So reported_iter always points to the last element that has been

shown to the user.

I'm fine with prev_iter as well,  What do you think?


Thanks

Best Regards

Hao


>>          (*pos)++;
>>          if (!ct)
>>                  return NULL;
>> --
>> 2.25.1
>>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-06-05  7:44 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-04  6:59 [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload Hao Ge
2026-06-04 19:24 ` Suren Baghdasaryan
2026-06-05  7:43   ` Hao Ge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox