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

[Hao Ge <hao.ge@linux.dev>]

On 2026/6/4 02:59, Suren Baghdasaryan wrote:
> On Mon, May 25, 2026 at 12:22 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.
>>
>> Fix by always reinitializing the iterator in allocinfo_start().
> Thanks for the fix!
> Yeah, unfortunately with the way seq_read_iter() is implemented, the
> op->next() fetches the next element and then seq_has_overflowed()
> might cause op->stop() without printing that element. So, in the
> op->start() that follows we can't just call codetag_next_ct(), which
> would handle the module unloading case (see
> https://elixir.bootlin.com/linux/v7.0.1/source/lib/codetag.c#L93).
>
>> Fixes: 9f44df50fee4 ("alloc_tag: keep codetag iterator active between read()")
>> Signed-off-by: Hao Ge <hao.ge@linux.dev>
>> ---
>>   lib/alloc_tag.c | 13 ++++++++-----
>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/alloc_tag.c b/lib/alloc_tag.c
>> index ed1bdcf1f8ab..2b2d1580c714 100644
>> --- a/lib/alloc_tag.c
>> +++ b/lib/alloc_tag.c
>> @@ -51,16 +51,19 @@ struct allocinfo_private {
>>   static void *allocinfo_start(struct seq_file *m, loff_t *pos)
>>   {
>>          struct allocinfo_private *priv;
>> +       struct codetag *ct;
>>          loff_t node = *pos;
>>
>>          priv = (struct allocinfo_private *)m->private;
>>          codetag_lock_module_list(alloc_tag_cttype, true);
>> -       if (node == 0) {
>> +       if (node == 0)
>>                  priv->print_header = true;
>> -               priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
>> -               codetag_next_ct(&priv->iter);
>> -       }
>> -       return priv->iter.ct ? priv : NULL;
>> +
>> +       priv->iter = codetag_get_ct_iter(alloc_tag_cttype);
>> +       while ((ct = codetag_next_ct(&priv->iter)) != NULL && node)
>> +               node--;
> I don't quite like this fix because if a module that we already passed
> gets unloaded too, then skipping "node" count of elements might skip
> some extra valid lines that we did not report yet. That's why
> codetag_next_ct() directly goes to the next module using
> idr_get_next_ul(). I would rather store the last codetag reported
> using allocinfo_show() (last_reported) and use
> codetag_next_ct(last_reported) in the allocinfo_start() to go to the
> next one (or possibly skip to the next module). Does that make sense?

Hi Suren

Thanks for the suggestion, I completely agree.

My patch has exactly the problem you described - reinitializing from 
scratch on every

start() also has an O(n) performance problem: each start(pos=N) needs to 
skip N

elements from the beginning, regardless of whether any module was unloaded.

One small difference from your suggestion: I save the iterator state in 
allocinfo_next() rather than allocinfo_show().

Since seq_read_iter() may roll back show()'s output on overflow

https://elixir.bootlin.com/linux/v7.1-rc6/source/fs/seq_file.c#L276

saving in show() could capture an element whose output was discarded, 
causing it to be skipped on the next start().

In the Fill loop, next() is not called again after a show() overflow 
since seq_read_iter() breaks out immediately.

Will send a v2 shortly.


Thanks

Best Regards

Hao

>> +
>> +       return ct ? priv : NULL;
>>   }
>>
>>   static void *allocinfo_next(struct seq_file *m, void *arg, loff_t *pos)
>> --
>> 2.25.1
>>