lkml.org 
[lkml]   [2002]   [Sep]   [21]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
/
DateSat, 21 Sep 2002 03:38:30 -0400
From"Kevin O'Connor" <>
SubjectRe: [PATCH] In-kernel module loader 1/7
On Fri, Sep 20, 2002 at 11:22:08AM +1000, Rusty Russell wrote:
> 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.

Hi Rusty,

Please consider the following non-module code snippet:

int
sys_enable_foo_security()
{
        foocache = kmalloc(100000);
        register_security(&foo_ops);
}
int
sys_disable_foo_security()
{
        unregister_security(&foo_ops);
        kfree(foocache);  // OOPS
}

If I follow Roman's argument correctly, the unload race is not module
specific.  (The problem is that unregister_security() only asserts that no
new callers will be made to foo_ops, it doesn't guarantee that there are no
current callers.)

In the above example, one solution would be to reference count foocache.
However, another viable solution would be to ref-count the security_ops
field.

Anyway, given that the problem is a general resource management issue (and
not module specific), I think one could implement call_security() with less
overhead:

#define call_security(method , ...)					\
	({ int __ret;							\
	   read_lock(&SecurityLock);                                    \
	   __ret = security_ops->method(__VA_ARGS__);                   \
           read_unlock(&SecurityLock);                                  \
	  __ret;							\
	})
where (un)register_security used a write_lock to guard accesses to
security_ops changes.

This implementation is still a bit sluggish (as well as limiting), however
one could conceivable use RCU or a similar mechanism to further reduce
overhead of the common path.

-Kevin


P.S. it may also be possible for this alternate solution to work:

#define call_security(method , ...)					\
	({ int __ret;							\
           atomic_inc(&SecurityRefCount);                               \
	   __ret = security_ops->method(__VA_ARGS__);                   \
           atomic_dec(&SecurityRefCount);                               \
	})
where unregister_security set the security_ops field to a dummy value and
then waited for the ref-count to hit zero before returning.  However, this
may depend too heavily on memory ordering..

-- 
 ------------------------------------------------------------------------
 | Kevin O'Connor                     "BTW, IMHO we need a FAQ for      |
 | kevin@koconnor.net                  'IMHO', 'FAQ', 'BTW', etc. !"    |
 ------------------------------------------------------------------------
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 12:29    [W:0.501 / U:0.100 seconds]
©2003-2008 Jasper Spaans