PROBLEM: SysV semaphore race vs SIGSTOP

PROBLEM: SysV semaphore race vs SIGSTOP

[Ove Kaaven]

There seem to be a race when SIGSTOP-ing a process waiting for a SysV
semaphore. Even if it could not possibly have owned the semaphore when
the signal was sent (because the sender of the signal owned it at the
time), it still occasionally happens that it both stops execution *and*
acquires the semaphore, with a deadlocked application as the result.
This is a problem for some of the high-performance stuff I'm working on.

A sample test program exhibiting the problem is available at
http://www.ping.uio.no/~ovehk/sembug.c

For me, it will show "ACQUIRE FAILED!! DEADLOCK!!" almost every time I
run it. Occasionally it will run fine; if it does for you, just try
again a couple of times.

The kernel I currently use is:

Linux version 2.4.27-1-k7 (horms@tabatha.lab.ultramonkey.org) (gcc
version 3.3.5 (Debian 1:3.3.5-2)) #1 Wed Dec 1 20:12:01 JST 2004

and I run it on a uniprocessor system (AMD Athlon, 1.9GHz) with Debian
"sid" installed.

I'm not a kernel hacker, but from a quick peruse of the 2.4 code, it
didn't seem to me like the semaphore code in the kernel (ipc/sem.c) even
try to handle suspended threads (though I wouldn't know how to do so).
The 2.6 semaphore code looked almost the same to me, too, so it might be
a problem there as well.

Please Cc me on any questions or comments, since I am too wimpy to
subscribe yet.

PATCH: SysV semaphore race vs SIGSTOP

[Ove Kaaven]

As I mentioned in an earlier mail, there is a race when SIGSTOP-ing a
process waiting for a SysV semaphore, where if a process holding a
semaphore suspends another process waiting on the semaphore and then
releases the semaphore, the suspended process might still get the
semaphore, resulting in a deadlock. My sample test program is at
http://www.ping.uio.no/~ovehk/sembug.c

Now I've written a patch for this which seems to work for me. It is
against 2.4.27, but the semaphore code doesn't seem to change often. And
please Cc me on any questions or comments, since I'm not subscribed.
Here is the patch:

--- ipc/sem.c.original	2005-01-31 18:17:17.000000000 -0500
+++ ipc/sem.c	2005-01-31 18:17:34.000000000 -0500
@@ -307,6 +307,18 @@
 	return result;
 }
 
+static int is_stopping(struct task_struct *t)
+{
+	if (t->state == TASK_STOPPED) {
+		/* shouldn't happen */
+		return 1;
+	}
+	if (sigismember(&t->pending.signal, SIGSTOP)) {
+		return 1;
+	}
+	return 0;
+}
+
 /* Go through the pending queue for the indicated semaphore
  * looking for tasks that can be completed.
  */
@@ -961,6 +973,12 @@
 			error = -EIDRM;
 			goto out_free;
 		}
+
+		if (is_stopping(current))
+			/* Could either EINTR out or continue.
+			 * Currently I've chosen to continue */
+			continue;
+
 		/*
 		 * If queue.status == 1 we where woken up and
 		 * have to retry else we simply return.