On Fri, Mar 6, 2026 at 7:43 AM Sami Tolvanen <[email protected]> wrote:
>
> On Wed, Dec 31, 2025 at 05:40:04PM +0800, Yafang Shao wrote:
> > --- a/kernel/module/main.c
> > +++ b/kernel/module/main.c
> > @@ -3901,7 +3901,11 @@ void print_modules(void)
> >       list_for_each_entry_rcu(mod, &modules, list) {
> >               if (mod->state == MODULE_STATE_UNFORMED)
> >                       continue;
> > -             pr_cont(" %s%s", mod->name, module_flags(mod, buf, true));
> > +             pr_cont(" %s", mod->name);
> > +             /* Only append version for out-of-tree modules */
> > +             if (mod->version && test_bit(TAINT_OOT_MODULE, &mod->taints))
> > +                     pr_cont("-%s", mod->version);
> > +             pr_cont("%s", module_flags(mod, buf, true));
>
> On second thought, is using mod->version here safe? We release the
> memory for mod->version in:
>
>   free_module
>     -> mod_sysfs_teardown
>     -> module_remove_modinfo_attrs
>     -> attr->free = free_modinfo_version
>
> And this happens before the module is removed from the
> list. Couldn't there be a race condition where we read a non-NULL
> mod->version here, but the buffer is being concurrently released
> by another core that's unloading the module, resulting in a
> use-after-free in the pr_cont call?

You're right. This can happen.

>
> In order to do this safely, we should presumably drop the attr->free
> call from module_remove_modinfo_attrs and release the attributes
> only after the synchronize_rcu call in free_module (there's already
> free_modinfo we can use), so mod->version is valid for the entire
> time the module is on the list.
>
> Thoughts?

It looks like this could work. I'll analyze it further—thanks for the
suggestion.

-- 
Regards
Yafang

Reply via email to