* Stephen Smalley (sdsat_private) wrote: > > I've attached a patch that adds documentation for the netdev > and module hooks. If there aren't any objections, I can commit > it. looks fine to me. > One question: I was reminded again of the fact that the > delete_module hook is only called in the case where a name is specified. > Do we want to also invoke it in the other case, with a NULL name? The > capable(CAP_SYS_MODULE) check is always performed, but I wondered if > that might not be adequate since CAP_SYS_MODULE is also used for all of > the other module calls. hmm, i think i like the idea of pushing the call to be inside free_module, but since it returns void maybe just before these calls. this gives the lsm module the opportunity to care about the actual module being unloaded, (in the null, module reaping case) not just the fact that _a_ module is being unloaded (which is already sort of protected, as you mentioned, by the capable(CAP_SYS_MODULE) call). passing the module structure to the delete_module call suggests the ability to tag a security attribute to the module struct (which i haven't added below). something like this (the NULL case needs some work -- what to do if error: continue, break, return 0, return error, etc --, this is just for conversation's sake): ===== kernel/module.c 1.14 vs edited ===== --- 1.14/kernel/module.c Thu Aug 16 12:27:23 2001 +++ edited/kernel/module.c Wed Sep 19 15:03:53 2001 @@ -627,13 +627,6 @@ if ((error = get_mod_name(name_user, &name)) < 0) goto out; - /* check that we have permission to do this */ - error = security_ops->module_ops->delete_module(name); - if (error) { - put_mod_name(name); - goto out; - } - error = -ENOENT; if ((mod = find_module(name)) == NULL) { put_mod_name(name); @@ -646,6 +639,12 @@ spin_lock(&unload_lock); if (!__MOD_IN_USE(mod)) { + /* check that we have permission to do this */ + error = security_ops->module_ops->delete_module(mod); + if (error) { + spin_unlock(&unload_lock); + goto out; + } mod->flags |= MOD_DELETED; spin_unlock(&unload_lock); free_module(mod, 0); @@ -674,6 +673,12 @@ spin_unlock(&unload_lock); mod->flags &= ~MOD_VISITED; } else { + /* check that we have permission to do this */ + error = security_ops->module_ops->delete_module(mod); + if (error) { + spin_unlock(&unload_lock); + continue; + } mod->flags |= MOD_DELETED; spin_unlock(&unload_lock); free_module(mod, 1); ===== include/linux/security.h 1.87 vs edited ===== --- 1.87/include/linux/security.h Wed Sep 19 12:05:41 2001 +++ edited/include/linux/security.h Wed Sep 19 14:59:41 2001 @@ -1647,16 +1647,17 @@ /** * delete_module - check permission when removing a module - * @name: module name + * @mod: module being deleted * * called: sys_delete_module <kernel/module.c> * * lock: The big kernel lock is held. + * lock: unload_lock is held. * * Check permission before removing a kernel module. * Return 0 if permission is granted. */ - int (* delete_module) (const char *name); + int (* delete_module) (const struct *module); }; /** ===== security/capability_plug.c 1.65 vs edited ===== --- 1.65/security/capability_plug.c Fri Sep 14 08:50:27 2001 +++ edited/security/capability_plug.c Wed Sep 19 14:52:22 2001 @@ -920,7 +920,7 @@ return 0; } -static int cap_module_delete_module (const char *name_user) +static int cap_module_delete_module (const struct module *mod) { return 0; } ===== security/dummy.c 1.3 vs edited ===== --- 1.3/security/dummy.c Fri Sep 14 08:50:27 2001 +++ edited/security/dummy.c Wed Sep 19 14:52:23 2001 @@ -741,7 +741,7 @@ return 0; } -static int dummy_module_delete_module (const char *name_user) +static int dummy_module_delete_module (const struct module *mod) { return 0; } _______________________________________________ linux-security-module mailing list linux-security-moduleat_private http://mail.wirex.com/mailman/listinfo/linux-security-module
This archive was generated by hypermail 2b30 : Wed Sep 19 2001 - 15:07:55 PDT