Race condition in tdb_runtime_check_for_robust_mutexes()

[Ralph Boehme]

On Thu, Mar 24, 2016 at 11:34:11PM +0200, Uri Simchoni wrote:
> On 03/24/2016 04:57 PM, Ralph Boehme wrote:
> > Something like the attached patch. This can probably be enhanced by
> > using pselect with a timeout and a signal mask to block/unblock
> > SIGCHLD instead of sleep.
> > 
> ENOPATCH?

gna, attached.

-Ralph

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de,mailto:kontakt@sernet.de
-------------- next part --------------
From 091cfad07a7693cb88f76e46f576fd39978bdc6f Mon Sep 17 00:00:00 2001
From: Ralph Boehme <slow at samba.org>
Date: Thu, 24 Mar 2016 15:49:13 +0100
Subject: [PATCH] WIP: only call waitpid() in the signal handler

---
 lib/tdb/common/mutex.c | 36 ++++++++++--------------------------
 1 file changed, 10 insertions(+), 26 deletions(-)

diff --git a/lib/tdb/common/mutex.c b/lib/tdb/common/mutex.c
index fae43d4..9556238 100644
--- a/lib/tdb/common/mutex.c
+++ b/lib/tdb/common/mutex.c
@@ -723,9 +723,6 @@ static bool tdb_robust_mutex_setup_sigchild(void (*handler)(int),
 	memset(&act, '\0', sizeof(act));
 
 	act.sa_handler = handler;
-#ifdef SA_RESTART
-	act.sa_flags = SA_RESTART;
-#endif
 	sigemptyset(&act.sa_mask);
 	sigaddset(&act.sa_mask, SIGCHLD);
 	sigaction(SIGCHLD, &act, &oldact);
@@ -884,17 +881,11 @@ _PUBLIC_ bool tdb_runtime_check_for_robust_mutexes(void)
 	}
 
 	while (tdb_robust_mutex_pid > 0) {
-		pid_t pid;
-
-		errno = 0;
-		pid = waitpid(tdb_robust_mutex_pid, &status, 0);
-		if (pid == tdb_robust_mutex_pid) {
-			tdb_robust_mutex_pid = -1;
-			break;
-		}
-		if (pid == -1 && errno != EINTR) {
-			goto cleanup_child;
-		}
+		/*
+		 * RACE: if the SIGCHLD is delivered now, *before* the
+		 * sleep, we will unneededly sleep for one second.
+		 */
+		sleep(1);
 	}
 	tdb_robust_mutex_setup_sigchild(tdb_robust_mutext_old_handler, NULL);
 
@@ -927,19 +918,12 @@ _PUBLIC_ bool tdb_runtime_check_for_robust_mutexes(void)
 
 cleanup_child:
 	while (tdb_robust_mutex_pid > 0) {
-		pid_t pid;
-
+		/*
+		 * RACE: if the SIGCHLD is delivered now, *before* the
+		 * sleep, we will unneededly sleep for one second.
+		 */
 		kill(tdb_robust_mutex_pid, SIGKILL);
-
-		errno = 0;
-		pid = waitpid(tdb_robust_mutex_pid, &status, 0);
-		if (pid == tdb_robust_mutex_pid) {
-			tdb_robust_mutex_pid = -1;
-			break;
-		}
-		if (pid == -1 && errno != EINTR) {
-			break;
-		}
+		sleep();
 	}
 cleanup_sig_child:
 	tdb_robust_mutex_setup_sigchild(tdb_robust_mutext_old_handler, NULL);
-- 
2.5.0