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

Kevin O'Connor (kevin@koconnor.net)
Sat, 21 Sep 2002 03:38:30 -0400


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/