Bug 51429: in safe_pclose, we may call kill + waitpid(WNOHANG) 1000 times

without giving the child any time to receive the SIGKILL signal and
terminate, break out of the while loop, and leave the child behind as a
zombie process.  The patch is contributed by John G. Myers
<jgmyers@speakeasy.net> and Tomas Mraz of Red Hat. r=neil.williams,relyea
 Tag: NSS_3_11_BRANCH

1 files changed, 12 insertions, 8 deletions

diff --git a/security/nss/lib/freebl/unix_rand.c b/security/nss/lib/freebl/unix_rand.c
index 13617b1e7..db94c2e09 100644
--- a/security/nss/lib/freebl/unix_rand.c
+++ b/security/nss/lib/freebl/unix_rand.c
@@ -46,6 +46,7 @@
 #include "secrng.h"
 #include "secerr.h"
 #include "prerror.h"
+#include "prthread.h"
 
 size_t RNG_FileUpdate(const char *fileName, size_t limit);
 
@@ -844,25 +845,28 @@ static int
 safe_pclose(FILE *fp)
 {
     pid_t pid;
-    int count, status;
+    int status = -1, rv;
 
     if ((pid = safe_popen_pid) == 0)
 	return -1;
     safe_popen_pid = 0;
 
+    fclose(fp);
+
+    /* yield the processor so the child gets some time to exit normally */
+    PR_Sleep(PR_INTERVAL_NO_WAIT);
+
     /* if the child hasn't exited, kill it -- we're done with its output */
-    count = 0;
-    while (waitpid(pid, &status, WNOHANG) == 0) {
-    	if (kill(pid, SIGKILL) < 0 && errno == ESRCH)
-	    break;
-	if (++count == 1000)
-	    break;
+    while ((rv = waitpid(pid, &status, WNOHANG)) == -1 && errno == EINTR)
+	;
+    if (rv == 0 && kill(pid, SIGKILL) == 0) {
+	while ((rv = waitpid(pid, &status, 0)) == -1 && errno == EINTR)
+	    ;
     }
 
     /* Reset SIGCHLD signal hander before returning */
     sigaction(SIGCHLD, &oldact, NULL);
 
-    fclose(fp);
     return status;
 }