Messages in this thread | | | From | Rusty Russell <> | Subject | Re: [PATCH] In-kernel module loader 1/7 | Date | Mon, 23 Sep 2002 09:31:42 +1000 |
| |
In message <20020921033830.A32446@arizona.localdomain> you write: > 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.)
But in practice there are many resources which are only unregistered in the "unloading module" case: certainly by far the most common case. It's hard for most module authors to spot this kind of race.
> 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; \ > })
Whack me with a cacheline... that's going to hurt on SMP. You could use a brlock, though.
You're assuming security methods cannot sleep? If true, use a brlock and be done with it. Otherwise, you'll need a refcount, and we're back to bigrefs and synchronize_kernel. Adding a bigref to each security_ops struct might be acceptable, since it's so big. Adding a single "owner" field certainly is.
Rusty. -- Anyone who quotes me in their sig is an idiot. -- Rusty Russell. - 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/
| |