net: fix multithreaded signal handling in unix recv routines

From: Rainer Weikusat <[email protected]>

The unix_dgram_recvmsg and unix_stream_recvmsg routines in net/af_unix.c utilize mutex_lock(&u->readlock) calls in order to serialize read operations of multiple threads on a single socket. This implies that, if all n threads of a process block in an AF_UNIX recv call trying to read data from the same socket, one of these threads will be sleeping in state TASK_INTERRUPTIBLE and all others in state TASK_UNINTERRUPTIBLE. Provided that a particular signal is supposed to be handled by a signal handler defined by the process and that none of this threads is blocking the signal, the complete_signal routine in kernel/signal.c will select the 'first' such thread it happens to encounter when deciding which thread to notify that a signal is supposed to be handled and if this is one of the TASK_UNINTERRUPTIBLE threads, the signal won't be handled until the one thread not blocking on the u->readlock mutex is woken up because some data to process has arrived (if this ever happens). The included patch fixes this by changing mutex_lock to mutex_lock_interruptible and handling possible error returns in the same way interruptions are handled by the actual receive-code.

---

diff -urp net-2.6/net/unix/af_unix.c net-2.6-patched//net/unix/af_unix.c
@@ -1724,7 +1724,11 @@ static int unix_dgram_recvmsg(struct kio
 
 	msg->msg_namelen = 0;
 
-	mutex_lock(&u->readlock);
+	err = mutex_lock_interruptible(&u->readlock);
+	if (err) {
+		err = sock_intr_errno(sock_rcvtimeo(sk, noblock));
+		goto out;
+	}
 
 	skb = skb_recv_datagram(sk, flags, noblock, &err);
 	if (!skb) {

@@ -1864,7 +1868,11 @@ static int unix_stream_recvmsg(struct ki
 		memset(&tmp_scm, 0, sizeof(tmp_scm));
 	}
 
-	mutex_lock(&u->readlock);
+	err = mutex_lock_interruptible(&u->readlock);
+	if (err) {
+		err = sock_intr_errno(timeo);
+		goto out;
+	}
 
 	do {
 		int chunk;

@@ -1895,11 +1903,12 @@ static int unix_stream_recvmsg(struct ki
 
 			timeo = unix_stream_data_wait(sk, timeo);
 
-			if (signal_pending(current)) {
+			if (signal_pending(current)
+			    ||  mutex_lock_interruptible(&u->readlock)) {
 				err = sock_intr_errno(timeo);
 				goto out;
 			}
-			mutex_lock(&u->readlock);
+
 			continue;
  unlock:
 			unix_state_unlock(sk);

This is IMHO a more sensible place for additional information.

I noticed this because the intended termination processing sequence of some program which is used as part of a 'content-filtering solution' for mobile devices (aka iPhones, iPads etc) stopped working the first time I tested the program in its intended 'actual execution context'. The program is supposed to listen for 'URL classifiction requests' on a AF_UNIX SOCK_SEQPACKET socket, pass these to a third-party library which does the actual classification job and then send a reply containing a list of categories associated with the URL in question. It uses multiple threads which basically work as follows:

1. initialize the library
2. unblock termination signals
3. block in read awaiting requests
4. block termination signals
5. process request and send reply
6. goto 2

Upon termination, each thread needs to execute the library finalization routine before exiting. This is supposed to work with the help of a signal handler for 'termination signals' calling siglongjmp to get the particular thread executing it out of the processing loop. Afterwards, this thread (with termination signals again blocked) does the finalization call, executes a kill(getpid(), SIGTERM) and exits via pthread_exit. The SIGTERM should then be picked up by another thread of the process which will then perform the same shutdown sequence and signal the next thread, until all threads of the process have terminated properly. An example program whose structure is basically identical to that of the actual application which demonstrates the problem is available here:

http://mss-uk.mssgmbh.com/~rw/signal/signal-problem-app.c

I've since spent some more thoughts on this and came to the conclusion that this should also affect independent process blocking on the same AF_UNIX socket and this even in absence of any user-defined signal handling. Another example program demonstrating this phenomenon can be downloaded from

http://mss-uk.mssgmbh.com/~rw/signal/signal-problem-fork-simple.c

This basically creates an 'unkillable' process, meaning, one which is even immune to a SIGKILL.

I've also tested that the issue still occurs with 2.6.38-rc5 and that it is fixed by the proposed patch. The program itself has meanwhile been moved to the computers which are actually used by the customers of my employer. This move included patching all the kernels running on these machines in the way I suggested.

From: Rainer Weikusat <[email protected]>
Date: Mon, 28 Feb 2011 14:50:55 +0000

Looks good, applied, thanks.