Re: [PATCH v2] module: add usage links when calling ref_module func

[Jessica Yu]

+++ Zhiqiang Liu [11/07/19 14:03 +0800]:
> On 2019/7/10 0:10, Jessica Yu wrote:
> > +++ Zhiqiang Liu [03/07/19 10:09 +0800]:
> > > From: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx
> > >
> > > V1->V2:
> > > - remove incorrect Fixes tag
> > > - fix error handling of sysfs_create_link as suggested by Jessica Yu
> > >
> > > Signed-off-by: Zhiqiang Liu <liuzhiqiang26@xxxxxxxxxx>
> > > Suggested-by: Jessica Yu <jeyu@xxxxxxxxxx>
> > > Reviewed-by: Kang Zhou <zhoukang7@xxxxxxxxxx>
> > > ---
> > > kernel/module.c | 18 ++++++++++++++----
> > > 1 file changed, 14 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/module.c b/kernel/module.c
> > > index 80c7c09584cf..672abce2222c 100644
> > > --- a/kernel/module.c
> > > +++ b/kernel/module.c
> > > @@ -837,25 +837,26 @@ static int already_uses(struct module *a, struct module *b)
> > >  *    'b' can walk the list to see who sourced them), and of 'a'
> > >  *    targets (so 'a' can see what modules it targets).
> > >  */
> > > /* Module a uses b: caller needs module_mutex() */
> > > int ref_module(struct module *a, struct module *b)
> > > {
> > > +    struct module_use *use;
> > >     int err;
> > >
> > >     if (b == NULL || already_uses(a, b))
> > > @@ -866,9 +867,18 @@ int ref_module(struct module *a, struct module *b)
> > >     if (err)
> > >         return err;
> > >
> > > -    err = add_module_usage(a, b);
> > > +    use = add_module_usage(a, b);
> > > +    if (!use) {
> > > +        module_put(b);
> > > +        return -ENOMEM;
> > > +    }
> > > +
> > > +    err = sysfs_create_link(b->holders_dir, &a->mkobj.kobj, a->name);
> >
> > Sigh. This ultimately doesn't work because in load_module(), we use
> > ref_module() in resolve_symbol(), and mod->mkobj.kobj doesn't get
> > initialized until mod_sysfs_init(), which happens much later in
> > load_module(). So what happens is that the ref_module(mod, owner) call
> > in resolve_symbol() returns an error because sysfs_create_link() fails here.
> > We could *maybe* move sysfs initialization earlier in load_module()
> > but that is an entirely untested idea and I would need to think about
> > that more.
>
> Thank you for the reply.
> I have tested the patch through livepatch. Maybe I miss somethings.
> I will rewrite the patch and test it entirely before sending the v3 patch.

Thanks Zhiqiang. A boot test and testing loading modules with
dependencies would be really appreciated.

Thanks!

Jessica