Re: [PATCH v2] alloc_tag: fix use-after-free in /proc/allocinfo after module unload

[Suren Baghdasaryan]

On Thu, Jun 4, 2026 at 12:00 AM Hao Ge <hao.ge@xxxxxxxxx> 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@xxxxxxxxxx>
> Signed-off-by: Hao Ge <hao.ge@xxxxxxxxx>

Acked-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>

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@xxxxxxxxx/
> ---
>  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
>