[PATCH 1/1] SYSVIPC - Fix the ipc structures initialization

[Nadia.Derbey@bull.net]

[-- Attachment #1: ipc-fix-sysvipc_structures_initialization.patch --]
[-- Type: text/plain, Size: 4579 bytes --]


This patch is a fix for Bugzilla bug
http://bugzilla.kernel.org/show_bug.cgi?id=11796.

To summarize, a simple testcase is concurrently running an infinite loop on
msgctl(IPC_STAT) and a call to msgget():

while (1)
   msgctl(id, IPC_STAT)    1
                           |
                           |
                           |
                           2 id = msgget(key, IPC_CREAT)
                           |
                           |
                           |

In the interval [1-2], the id doesn't exist yet.

In that test, the problem is the following:
When we are calling ipc_addid() from msgget() the msq structure is not
completely initialized. So idr_get_new() is inserting a pointer into the
idr tree, and the structure which is pointed to has, among other fields,
its lock uninitialized.

Since msgctl(IPC_STAT) is looping while (1), idr_find() returns the
pointer as soon as it is inserted into the IDR tree. And ipc_lock()
calls spin_lock(&mqs->lock), while we have not initialized that lock
yet.

This patch moves the spin_lock_init() before the call to ipc_addid().
It also sets the "deleted" flag to 1 in the window between msg structure
allocation and msg structure locking in ipc_addid().

The fix has been applied to semaphores and shm segments too.

Regards,
Nadia


Signed-off-by: Nadia Derbey <Nadia.Derbey@bull.net>

---
 ipc/msg.c  |    9 +++++++++
 ipc/sem.c  |    9 +++++++++
 ipc/shm.c  |    9 +++++++++
 ipc/util.c |    3 ++-
 4 files changed, 29 insertions(+), 1 deletion(-)

Index: linux-2.6.27/ipc/msg.c
===================================================================
--- linux-2.6.27.orig/ipc/msg.c	2008-10-23 15:20:46.000000000 +0200
+++ linux-2.6.27/ipc/msg.c	2008-10-23 16:38:59.000000000 +0200
@@ -191,6 +191,15 @@ static int newque(struct ipc_namespace *
 	msq->q_perm.mode = msgflg & S_IRWXUGO;
 	msq->q_perm.key = key;
 
+	/*
+	 * We have a window between the time msq is inserted into the idr
+	 * tree (done in ipc_addid()) and the time it is actually locked.
+	 * In order to be safe during that window set the msq as deleted:
+	 * a concurrent ipc_lock() will see it as not present until the
+	 * initialization phase is complete.
+	 */
+	msq->q_perm.deleted = 1;
+
 	msq->q_perm.security = NULL;
 	retval = security_msg_queue_alloc(msq);
 	if (retval) {
Index: linux-2.6.27/ipc/util.c
===================================================================
--- linux-2.6.27.orig/ipc/util.c	2008-10-23 15:20:46.000000000 +0200
+++ linux-2.6.27/ipc/util.c	2008-10-23 15:33:37.000000000 +0200
@@ -266,6 +266,8 @@ int ipc_addid(struct ipc_ids* ids, struc
 	if (ids->in_use >= size)
 		return -ENOSPC;
 
+	spin_lock_init(&new->lock);
+
 	err = idr_get_new(&ids->ipcs_idr, new, &id);
 	if (err)
 		return err;
@@ -280,7 +282,6 @@ int ipc_addid(struct ipc_ids* ids, struc
 		ids->seq = 0;
 
 	new->id = ipc_buildid(id, new->seq);
-	spin_lock_init(&new->lock);
 	new->deleted = 0;
 	rcu_read_lock();
 	spin_lock(&new->lock);
Index: linux-2.6.27/ipc/sem.c
===================================================================
--- linux-2.6.27.orig/ipc/sem.c	2008-10-23 15:20:46.000000000 +0200
+++ linux-2.6.27/ipc/sem.c	2008-10-23 15:57:27.000000000 +0200
@@ -256,6 +256,15 @@ static int newary(struct ipc_namespace *
 	sma->sem_perm.mode = (semflg & S_IRWXUGO);
 	sma->sem_perm.key = key;
 
+	/*
+	 * We have a window between the time sma is inserted into the idr
+	 * tree (done in ipc_addid()) and the time it is actually locked.
+	 * In order to be safe during that window set the sma as deleted:
+	 * a concurrent ipc_lock() will see it as not present until the
+	 * initialization phase is complete.
+	 */
+	sma->sem_perm.deleted = 1;
+
 	sma->sem_perm.security = NULL;
 	retval = security_sem_alloc(sma);
 	if (retval) {
Index: linux-2.6.27/ipc/shm.c
===================================================================
--- linux-2.6.27.orig/ipc/shm.c	2008-10-23 15:20:46.000000000 +0200
+++ linux-2.6.27/ipc/shm.c	2008-10-23 15:56:32.000000000 +0200
@@ -355,6 +355,15 @@ static int newseg(struct ipc_namespace *
 	shp->shm_perm.mode = (shmflg & S_IRWXUGO);
 	shp->mlock_user = NULL;
 
+	/*
+	 * We have a window between the time shp is inserted into the idr
+	 * tree (done in ipc_addid()) and the time it is actually locked.
+	 * In order to be safe during that window set the shp as deleted:
+	 * a concurrent ipc_lock() will see it as not present until the
+	 * initialization phase is complete.
+	 */
+	shp->shm_perm.deleted = 1;
+
 	shp->shm_perm.security = NULL;
 	error = security_shm_alloc(shp);
 	if (error) {

--