Re: [PATCH] module interface improvement for kprobes

[Chuck Ebbert <76306.1226@compuserve.com>]

In-Reply-To: <1154721061.15967.31.camel@dhcp-2.hsv.redhat.com>

On Fri, 04 Aug 2006 14:51:01 -0500, David Smith wrote:

> > > I've added a new function, module_get_byname(), that looks up a module
> > > by name and returns the module reference.  Note that module_get_byname()
> > > also increments the module reference count.  It does this so that the
> > > module won't be unloaded between the time that module_get_byname() is
> > > called and register_kprobe() is called.

Your patch is word-wrapped.

Also:

> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -291,7 +291,27 @@ static struct module *find_module(const 
>       }
>       return NULL;
>  }
> +  
> +int module_get_byname(const char *mod_name, struct module **mod)
> +{
> +     *mod = NULL;
>  
> +     /* We must hold module_mutex before calling find_module(). */
> +     if (mutex_lock_interruptible(&module_mutex) != 0)
> +             return -EINTR;
> +
> +     *mod = find_module(mod_name);
> +     mutex_unlock(&module_mutex);
> +     if (*mod) {
> +             if (!strong_try_module_get(*mod)) {

What keeps the module from going away between the mutex_unlock() and
strong_try_module_get()? It needs to be something like this:

        *mod = find_module(mod_name);
        if (*mod)
                ret = strong_try_module_get(*mod);
        mutex_unlock(&module_mutex);
        if (!ret) {
                *mod = NULL;
                return -EINVAL;
        }

> +                     *mod = NULL;
> +                     return -EINVAL;
> +             }
> +     }
> +     return 0;
> +}
> +EXPORT_SYMBOL_GPL(module_get_byname);
> + 
>  #ifdef CONFIG_SMP
>  /* Number of blocks used and allocated. */
>  static unsigned int pcpu_num_used, pcpu_num_allocated;

-- 
Chuck