Re: [PATCH] In-kernel module loader 1/7

[Rusty Russell <rusty@rustcorp.com.au>]

In message <20020919183843.GA16568@kroah.com> you write:
> On Thu, Sep 19, 2002 at 03:54:40PM +0200, Roman Zippel wrote:
> > I already said often enough, a module has only to answer the simple
> > question: Is it safe to unload the module?
> 
> And with a LSM module, how can it answer that?  There's no way, unless
> we count every time someone calls into our module.  And if you do that,
> no one will even want to use your module, given the number of hooks, and
> the paths those hooks are on (the speed hit would be horrible.)

Well, it's up to you.  You *could* implement:

#define call_security(method , ...)					\
	({ int __ret;							\
	   if (try_module_get(security_ops->owner)) {			\
		__ret = security_ops->method(__VA_ARGS__);		\
		module_put(security_ops->owner);			\
	   } else							\
		/* If unloading or loading, default to "allow" */	\
		__ret = 0;						\
	  __ret;							\
	})

Now, if you don't have CONFIG_MODULES this becomes the code as it is
now.  If you don't have CONFIG_MODULE_UNLOAD, this becomes:

	if (security_ops->owner && security_ops->owner->live)
		__ret = security_ops->method(__VA_ARGS__);
	else
		__ret = 0;

With the case of CONFIG_MODULE_UNLOAD, it looks *really* horrible (53
instructions with 6 branches: ewww...).  So I would recommend
register_security insert a shim and take the hit itself:

#ifdef CONFIG_MODULE_UNLOAD
	if (ops->owner) {
		module_ops->real = ops;
		ops = module_ops;
	}
#endif

Then module_ops does the inc/dec wrappers (well, it can optimize if
the ops can't sleep...).

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.