From: Paul Mackerras <paulus@samba.org>
Date: Sun, 4 Jun 2006 03:52:50 +0000 (+0000)
Subject: Fix snafu in run_program().  Patch from Robert Vogelgesang.
X-Git-Tag: ppp-2.4.7~116
X-Git-Url: https://git.ozlabs.org/?a=commitdiff_plain;ds=inline;h=d929a8d4e1d3e5ff15068ea3b538598b6d448943;p=ppp.git

Fix snafu in run_program().  Patch from Robert Vogelgesang.

This patch splits out the major part of reap_kids() into a new
function, forget_child(), and calls this new function instead of
reap_kids() in run_program(), after having waitpid()'ed for a child.

Rationale:
The waitpid() at the start of reap_kids() has no chance to get the
PID of the child already waited for in run_program().
As a consequence, the PID of that child will stay in the list of all
children until the end of the pppd process, which is bad, because
pppd will then try to kill -TERM that PID (and might kill some
innocent third party at this point).
---

diff --git a/pppd/main.c b/pppd/main.c
index fe34cff..9c4cb46 100644
--- a/pppd/main.c
+++ b/pppd/main.c
@@ -66,7 +66,7 @@
  * OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  */
 
-#define RCSID	"$Id: main.c,v 1.152 2005/08/25 23:59:34 paulus Exp $"
+#define RCSID	"$Id: main.c,v 1.153 2006/06/04 03:52:50 paulus Exp $"
 
 #include <stdio.h>
 #include <ctype.h>
@@ -242,6 +242,7 @@ static void toggle_debug __P((int));
 static void open_ccp __P((int));
 static void bad_signal __P((int));
 static void holdoff_end __P((void *));
+static void forget_child __P((int pid, int status));
 static int reap_kids __P((void));
 static void childwait_end __P((void *));
 
@@ -1711,7 +1712,7 @@ run_program(prog, args, must_exist, done, arg, wait)
 		    continue;
 		fatal("error waiting for script %s: %m", prog);
 	    }
-	    reap_kids();
+	    forget_child(pid, status);
 	}
 	return pid;
     }
@@ -1788,6 +1789,35 @@ childwait_end(arg)
     childwait_done = 1;
 }
 
+/*
+ * forget_child - clean up after a dead child
+ */
+static void
+forget_child(pid, status)
+    int pid, status;
+{
+    struct subprocess *chp, **prevp;
+
+    for (prevp = &children; (chp = *prevp) != NULL; prevp = &chp->next) {
+        if (chp->pid == pid) {
+	    --n_children;
+	    *prevp = chp->next;
+	    break;
+	}
+    }
+    if (WIFSIGNALED(status)) {
+        warn("Child process %s (pid %d) terminated with signal %d",
+	     (chp? chp->prog: "??"), pid, WTERMSIG(status));
+    } else if (debug)
+        dbglog("Script %s finished (pid %d), status = 0x%x",
+	       (chp? chp->prog: "??"), pid,
+	       WIFEXITED(status) ? WEXITSTATUS(status) : status);
+    if (chp && chp->done)
+        (*chp->done)(chp->arg);
+    if (chp)
+        free(chp);
+}
+
 /*
  * reap_kids - get status from any dead child processes,
  * and log a message for abnormal terminations.
@@ -1796,29 +1826,11 @@ static int
 reap_kids()
 {
     int pid, status;
-    struct subprocess *chp, **prevp;
 
     if (n_children == 0)
 	return 0;
     while ((pid = waitpid(-1, &status, WNOHANG)) != -1 && pid != 0) {
-	for (prevp = &children; (chp = *prevp) != NULL; prevp = &chp->next) {
-	    if (chp->pid == pid) {
-		--n_children;
-		*prevp = chp->next;
-		break;
-	    }
-	}
-	if (WIFSIGNALED(status)) {
-	    warn("Child process %s (pid %d) terminated with signal %d",
-		 (chp? chp->prog: "??"), pid, WTERMSIG(status));
-	} else if (debug)
-	    dbglog("Script %s finished (pid %d), status = 0x%x",
-		   (chp? chp->prog: "??"), pid,
-		   WIFEXITED(status) ? WEXITSTATUS(status) : status);
-	if (chp && chp->done)
-	    (*chp->done)(chp->arg);
-	if (chp)
-	    free(chp);
+        forget_child(pid, status);
     }
     if (pid == -1) {
 	if (errno == ECHILD)