Re: [PATCH] static_call: Handle module init failure correctly in static_call_del_module()

From: Luis Chamberlain
Date: Thu Sep 19 2024 - 05:58:15 EST


On Fri, Sep 06, 2024 at 04:24:56PM -0700, Luis Chamberlain wrote:
> On Thu, Sep 05, 2024 at 11:44:00AM +0200, Thomas Gleixner wrote:
> > Now you at least provided the information that the missing cleanup in
> > the init() function is not the problem. So the obvious place to look is
> > in the module core code whether there is a failure path _after_
> > module->init() returned success.
> >
> > do_init_module()
> > ret = do_one_initcall(mod->init);
> > ...
> > ret = module_enable_rodata_ro(mod, true);
> > if (ret)
> > goto fail_mutex_unlock;
> >
> > and that error path does _not_ invoke module->exit(), which is obviously
> > not correct. Luis?
>
> You're spot on this needs fixing.

Christophe, this is a regression caused by the second hunk of your commit
d1909c0221739 ("module: Don't ignore errors from set_memory_XX()") on v6.9.
Sadly there are a few issues with trying to get to call mod->exit():

- We should try try_stop_module() and that can fail
- source_list may not be empty and that would block removal
- mod->exit may not exist

I'm wondering if instead we should try to do the module_enable_rodata_ro()
before the init, but that requires a bit more careful evaluation...

Luis