public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Vasiliy Kulikov <segoon@openwall.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	Manuel Lauss <manuel.lauss@googlemail.com>,
	linux-kernel@vger.kernel.org, Richard Weinberger <richard@nod.at>,
	Marc Zyngier <maz@misterjones.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>,
	kernel-hardening@lists.openwall.com,
	"Paul E. McKenney" <paul.mckenney@linaro.org>
Subject: [PATCH] shm: fix a race between shm_exit() and shm_init()
Date: Tue, 2 Aug 2011 16:45:30 +0400	[thread overview]
Message-ID: <20110802124530.GA2543@albatros> (raw)
In-Reply-To: <CA+55aFw2kHWY1N=ox1er7c-3WL4OA8x0M=Keu7hvAncKUZd1tQ@mail.gmail.com>

Hi,

Manuel, Richard, Marc - can you test this patch, please?


From: Vasiliy Kulikov <segoon@openwall.com>
Subject: [PATCH] shm: fix a race between shm_exit() and shm_init()

On thread exit shm_exit() is called, it uses shm_ids(ns).rw_mutex.
It is initialized in shm_init(), but it is not called yet at the moment
of kernel threads exit.  Some kernel threads are created in
do_pre_smp_initcalls(), and shm_init() is called in do_initcalls().

Static initialization of shm_ids(init_ipc_ns).rw_mutex fixes the race.

It fixes a kernel oops:

Unable to handle kernel NULL pointer dereference at virtual address 00000000
...
[<c0320090>] (__down_write_nested+0x88/0xe0) from [<c015da08>] (exit_shm+0x28/0x48)
[<c015da08>] (exit_shm+0x28/0x48) from [<c002e550>] (do_exit+0x59c/0x750)
[<c002e550>] (do_exit+0x59c/0x750) from [<c003eaac>] (____call_usermodehelper+0x13c/0x154)
[<c003eaac>] (____call_usermodehelper+0x13c/0x154) from [<c000f630>] (kernel_thread_exit+0x0/0x8)
Code: 1afffffa e597c00c e58d0000 e587d00c (e58cd000)

Reported-by: Manuel Lauss <manuel.lauss@googlemail.com>
Reported-by: Richard Weinberger <richard@nod.at>
Reported-by: Marc Zyngier <maz@misterjones.org>
Signed-off-by: Vasiliy Kulikov <segoon@openwall.com>
---
 ipc/msgutil.c |    6 ++++++
 ipc/shm.c     |   11 ++++++++++-
 ipc/util.c    |   30 +++++++++++++++++-------------
 ipc/util.h    |    1 +
 4 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index 8b5ce5d..6da67b6 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -20,6 +20,9 @@
 
 DEFINE_SPINLOCK(mq_lock);
 
+#define INIT_IPC_SHM_IDS(name) \
+	{ .rw_mutex = __RWSEM_INITIALIZER(name.rw_mutex), }
+
 /*
  * The next 2 defines are here bc this is the only file
  * compiled when either CONFIG_SYSVIPC and CONFIG_POSIX_MQUEUE
@@ -27,6 +30,9 @@ DEFINE_SPINLOCK(mq_lock);
  */
 struct ipc_namespace init_ipc_ns = {
 	.count		= ATOMIC_INIT(1),
+	.ids	= {
+		[IPC_SHM_IDS] = INIT_IPC_SHM_IDS(init_ipc_ns.ids[IPC_SHM_IDS]),
+	},
 #ifdef CONFIG_POSIX_MQUEUE
 	.mq_queues_max   = DFLT_QUEUESMAX,
 	.mq_msg_max      = DFLT_MSGMAX,
diff --git a/ipc/shm.c b/ipc/shm.c
index 9fb044f..7d084a0 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -76,7 +76,16 @@ void shm_init_ns(struct ipc_namespace *ns)
 	ns->shm_ctlmni = SHMMNI;
 	ns->shm_rmid_forced = 0;
 	ns->shm_tot = 0;
-	ipc_init_ids(&shm_ids(ns));
+
+	/*
+	 * 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));
 }
 
 /*
diff --git a/ipc/util.c b/ipc/util.c
index 75261a3..673dde5 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -108,31 +108,35 @@ static int __init ipc_init(void)
 }
 __initcall(ipc_init);
 
-/**
- *	ipc_init_ids		-	initialise IPC identifiers
- *	@ids: Identifier set
- *
- *	Set up the sequence range to use for the ipc identifier range (limited
- *	below IPCMNI) then initialise the ids idr.
- */
- 
-void ipc_init_ids(struct ipc_ids *ids)
+void __ipc_init_ids(struct ipc_ids *ids)
 {
-	init_rwsem(&ids->rw_mutex);
-
 	ids->in_use = 0;
 	ids->seq = 0;
 	{
 		int seq_limit = INT_MAX/SEQ_MULTIPLIER;
 		if (seq_limit > USHRT_MAX)
 			ids->seq_max = USHRT_MAX;
-		 else
-		 	ids->seq_max = seq_limit;
+		else
+			ids->seq_max = seq_limit;
 	}
 
 	idr_init(&ids->ipcs_idr);
 }
 
+/**
+ *	ipc_init_ids		-	initialise IPC identifiers
+ *	@ids: Identifier set
+ *
+ *	Set up the sequence range to use for the ipc identifier range (limited
+ *	below IPCMNI) then initialise the ids idr.
+ */
+
+void ipc_init_ids(struct ipc_ids *ids)
+{
+	init_rwsem(&ids->rw_mutex);
+	__ipc_init_ids(ids);
+}
+
 #ifdef CONFIG_PROC_FS
 static const struct file_operations sysvipc_proc_fops;
 /**
diff --git a/ipc/util.h b/ipc/util.h
index 6f5c20b..8bbcd9c 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -80,6 +80,7 @@ struct seq_file;
 struct ipc_ids;
 
 void ipc_init_ids(struct ipc_ids *);
+void __ipc_init_ids(struct ipc_ids *ids);
 #ifdef CONFIG_PROC_FS
 void __init ipc_init_proc_interface(const char *path, const char *header,
 		int ids, int (*show)(struct seq_file *, void *));
-- 
1.7.0.4


  reply	other threads:[~2011-08-02 12:45 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-08-01 18:01 initcall dependency problem (ns vs. threads) Vasiliy Kulikov
2011-08-01 18:20 ` Andrew Morton
2011-08-01 18:34   ` [kernel-hardening] " Vasiliy Kulikov
2011-08-01 19:03   ` Vasiliy Kulikov
2011-08-01 19:07     ` Andrew Morton
2011-08-01 19:22       ` Vasiliy Kulikov
2011-08-02  0:01     ` Linus Torvalds
2011-08-02 12:45       ` Vasiliy Kulikov [this message]
2011-08-02 12:51         ` [PATCH] shm: fix a race between shm_exit() and shm_init() Manuel Lauss
2011-08-02 13:23         ` Richard Weinberger
2011-08-02 13:29         ` Marc Zyngier
2011-08-02 20:33         ` Andrew Morton
2011-08-02 20:55         ` Andrew Morton
2011-08-03  5:30           ` Manuel Lauss
2011-08-03  8:05           ` Marc Zyngier
2011-08-03  8:19             ` Linus Torvalds
2011-08-03 10:04               ` Manuel Lauss
2011-08-03 10:30               ` Marc Zyngier
2011-08-03 13:13                 ` Thadeu Lima de Souza Cascardo
2011-08-03 13:33                   ` Kay Sievers
2011-08-03 13:45                     ` Richard Weinberger
2011-08-04  0:35                 ` Linus Torvalds
2011-08-04  0:50                   ` Andrew Morton
2011-08-04  1:01                     ` Linus Torvalds
2011-08-04  1:15                       ` Kay Sievers
2011-08-04  8:26                   ` Marc Zyngier
2011-08-03  7:43         ` Linus Torvalds
2011-08-03  7:50           ` Manuel Lauss
2011-08-03  8:00             ` Manuel Lauss
2011-08-03 19:33           ` Andrew Morton
2011-08-03 19:52             ` [kernel-hardening] " Vasiliy Kulikov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110802124530.GA2543@albatros \
    --to=segoon@openwall.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=manuel.lauss@googlemail.com \
    --cc=maz@misterjones.org \
    --cc=mingo@elte.hu \
    --cc=paul.mckenney@linaro.org \
    --cc=richard@nod.at \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox