[kernel-hardening] Re: [PATCH] shm: fix a race between shm_exit() and shm_init()

Linus Torvalds
2011-08-03T07:43:23Z

[-- Attachment #1: Type: text/plain, Size: 1222 bytes --]

On Tue, Aug 2, 2011 at 2:45 AM, Vasiliy Kulikov <segoon@openwall.com> wrote:
>
> From: Vasiliy Kulikov <segoon@openwall.com>
> Subject: [PATCH] shm: fix a race between shm_exit() and shm_init()

This patch is disgusting.

Doing things like this:

> +       /*
> +        * For init_ipc_ns shm_ids().rw_mutex is statically initialized
> +        * as kernel threads should be able to use it in do_exit() before
> +        * shm_init(), which is called on do_initcall()
> +        */
> +       if (ns == &init_ipc_ns)
> +               __ipc_init_ids(&shm_ids(ns));
> +       else
> +               ipc_init_ids(&shm_ids(ns));

should have told you that there is something totally wrong with your patch.

I'd prefer to really do the initialization in the allocator (at which
point it would be very natural to do the initialization statically for
a static allocation, and you wouldn't have the above kind of nasty
conditional stuff), but that whole namespace initialization and setup
just looks pretty nasty.

Looking at some of the other cases like net_ns_init(), maybe the
proper fix is to just make 'ipc_ns_init()' be a pure_initcall().

Does the attached patch work?

Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 654 bytes --]

ipc/shm.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/ipc/shm.c b/ipc/shm.c
index 9fb044f3b345..3791fd865bbd 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -105,9 +105,16 @@ void shm_exit_ns(struct ipc_namespace *ns)
}
#endif

-void __init shm_init (void)
+static int __init ipc_ns_init(void)
{
shm_init_ns(&init_ipc_ns);
+ return 0;
+}
+
+pure_initcall(ipc_ns_init);
+
+void __init shm_init (void)
+{
ipc_init_proc_interface("sysvipc/shm",
#if BITS_PER_LONG <= 32
" key shmid perms size cpid lpid nattch uid gid cuid cgid atime dtime ctime rss swap\n",