Messages in this thread | | | From | Julien Thierry <> | Subject | Re: [POC 01/23] module: Allow to delete module also from inside kernel | Date | Tue, 21 Jan 2020 11:11:38 +0000 |
| |
Hi Petr,
On 1/17/20 3:03 PM, Petr Mladek wrote: > Livepatch subsystems will need to automatically load and delete > livepatch module when the livepatched one is being removed > or when the entire livepatch is being removed. > > The code stopping the kernel module is moved to separate function > so that it can be reused. > > The function always have rights to do the action. Also it does not > need to search for struct module because it is already passed as > a parameter. > > On the other hand, it has to make sure that the given struct module > can't be released in parallel. It is achieved by combining module_put() > and module_delete() functionality in a single function. > > This patch does not change the existing behavior of delete_module > syscall. > > Signed-off-by: Petr Mladek <pmladek@suse.com> > > module: Add module_put_and_delete() > --- > include/linux/module.h | 5 +++ > kernel/module.c | 119 +++++++++++++++++++++++++++++++------------------ > 2 files changed, 80 insertions(+), 44 deletions(-) > > diff --git a/include/linux/module.h b/include/linux/module.h > index bd165ba68617..f69f3fd72dd5 100644 > --- a/include/linux/module.h > +++ b/include/linux/module.h > @@ -623,6 +623,7 @@ extern void __module_get(struct module *module); > extern bool try_module_get(struct module *module); > > extern void module_put(struct module *module); > +extern int module_put_and_delete(struct module *mod); > > #else /*!CONFIG_MODULE_UNLOAD*/ > static inline bool try_module_get(struct module *module) > @@ -632,6 +633,10 @@ static inline bool try_module_get(struct module *module) > static inline void module_put(struct module *module) > { > } > +static inline int module_put_and_delete(struct module *mod) > +{ > + return 0; > +} > static inline void __module_get(struct module *module) > { > } > diff --git a/kernel/module.c b/kernel/module.c > index b56f3224b161..b3f11524f8f9 100644 > --- a/kernel/module.c > +++ b/kernel/module.c > @@ -964,62 +964,36 @@ EXPORT_SYMBOL(module_refcount); > /* This exists whether we can unload or not */ > static void free_module(struct module *mod); > > -SYSCALL_DEFINE2(delete_module, const char __user *, name_user, > - unsigned int, flags) > +int stop_module(struct module *mod, int flags) > { > - struct module *mod; > - char name[MODULE_NAME_LEN]; > - int ret, forced = 0; > - > - if (!capable(CAP_SYS_MODULE) || modules_disabled) > - return -EPERM; > - > - if (strncpy_from_user(name, name_user, MODULE_NAME_LEN-1) < 0) > - return -EFAULT; > - name[MODULE_NAME_LEN-1] = '\0'; > + int forced = 0; > > - audit_log_kern_module(name); > - > - if (mutex_lock_interruptible(&module_mutex) != 0) > - return -EINTR; > - > - mod = find_module(name); > - if (!mod) { > - ret = -ENOENT; > - goto out; > - } > - > - if (!list_empty(&mod->source_list)) { > - /* Other modules depend on us: get rid of them first. */ > - ret = -EWOULDBLOCK; > - goto out; > - } > + /* Other modules depend on us: get rid of them first. */ > + if (!list_empty(&mod->source_list)) > + return -EWOULDBLOCK; > > /* Doing init or already dying? */ > if (mod->state != MODULE_STATE_LIVE) { > /* FIXME: if (force), slam module count damn the torpedoes */ > pr_debug("%s already dying\n", mod->name); > - ret = -EBUSY; > - goto out; > + return -EBUSY; > } > > /* If it has an init func, it must have an exit func to unload */ > if (mod->init && !mod->exit) { > forced = try_force_unload(flags); > - if (!forced) { > - /* This module can't be removed */ > - ret = -EBUSY; > - goto out; > - } > + /* This module can't be removed */ > + if (!forced) > + return -EBUSY; > } > > /* Stop the machine so refcounts can't move and disable module. */ > - ret = try_stop_module(mod, flags, &forced); > - if (ret != 0) > - goto out; > + return try_stop_module(mod, flags, &forced); > +} > > - mutex_unlock(&module_mutex); > - /* Final destruction now no one is using it. */ > +/* Final destruction now no one is using it. */
Nit: Looks like some copy/paste mixup
> +static void destruct_module(struct module *mod) > +{ > if (mod->exit != NULL) > mod->exit(); > blocking_notifier_call_chain(&module_notify_list, > @@ -1033,8 +1007,44 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user, > strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module)); > > free_module(mod); > + > /* someone could wait for the module in add_unformed_module() */ > wake_up_all(&module_wq); > +} > + > +SYSCALL_DEFINE2(delete_module, const char __user *, name_user, > + unsigned int, flags) > +{ > + struct module *mod; > + char name[MODULE_NAME_LEN]; > + int ret; > + > + if (!capable(CAP_SYS_MODULE) || modules_disabled) > + return -EPERM; > + > + if (strncpy_from_user(name, name_user, MODULE_NAME_LEN-1) < 0) > + return -EFAULT; > + name[MODULE_NAME_LEN-1] = '\0'; > + > + audit_log_kern_module(name); > + > + if (mutex_lock_interruptible(&module_mutex) != 0) > + return -EINTR; > + > + mod = find_module(name); > + if (!mod) { > + ret = -ENOENT; > + goto out; > + } > + > + ret = stop_module(mod, flags); > + if (ret) > + goto out; > + > + mutex_unlock(&module_mutex); > + > +/* Final destruction now no one is using it. */ > + destruct_module(mod); > return 0; > out: > mutex_unlock(&module_mutex); > @@ -1138,20 +1148,41 @@ bool try_module_get(struct module *module) > } > EXPORT_SYMBOL(try_module_get); > > -void module_put(struct module *module) > +/* Must be called under module_mutex or with preemtion disabled */
Might be worth adding some asserts to check for that.
> +static void __module_put(struct module* module) > { > int ret; > > + ret = atomic_dec_if_positive(&module->refcnt); > + WARN_ON(ret < 0); /* Failed to put refcount */ > + trace_module_put(module, _RET_IP_); > +} > + > +void module_put(struct module *module) > +{ > if (module) { > preempt_disable(); > - ret = atomic_dec_if_positive(&module->refcnt); > - WARN_ON(ret < 0); /* Failed to put refcount */ > - trace_module_put(module, _RET_IP_); > + __module_put(module); > preempt_enable(); > } > } > EXPORT_SYMBOL(module_put); > > +int module_put_and_delete(struct module *mod) > +{ > + int ret; > + mutex_lock(&module_mutex); > + __module_put(mod); > + ret = stop_module(mod, 0); > + mutex_unlock(&module_mutex); > + > + if (ret) > + return ret; > + > + destruct_module(mod); > + return 0; > +} > + > #else /* !CONFIG_MODULE_UNLOAD */ > static inline void print_unload_info(struct seq_file *m, struct module *mod) > { >
Thanks,
-- Julien Thierry
| |