[PATCH] rsync kills all user processes on fork failure

Bob Byrnes byrnes at curl.com
Thu May 9 19:39:06 EST 2002


On May 9,  4:51pm, dwd at bell-labs.com (Dave Dykstra) wrote:
-- Subject: Re: [PATCH] rsync kills all user processes on fork failure
>
> What's the best fix against the current CVS?  Should we back out the
> previous fix because that one only solved half the problem?  Somebody
> please provide an updated patch.
> 
-- End of excerpt from Dave Dykstra

I've appended an updated patch that can be applied to the current CVS.

Description:

	--  Be extra careful: reject any negative pid in do_fork(),
	    not just -1 (since other negative values would also be
	    harmful if they were ever passed to kill, even though
	    such values should never occur).

	--  Expand all_pids[] dynamically as needed in do_fork(), to
	    avoid any possibility of overflow.

	--  Prevent kill(-1, SIGUSR2) in main.c; this is independent of
	    the interaction between do_fork() and kill_all(), which
	    involves SIGUSR1.

Bob Byrnes                        e-mail: byrnes at curl.com
Curl Corporation                  phone:  617-761-1200
400 Technology Square, 8th Floor  fax:    617-761-1201
Cambridge, MA  02139

--- util.c.orig	Thu May  9 22:12:37 2002
+++ util.c	Thu May  9 22:15:20 2002
@@ -373,15 +373,25 @@
 }
 
 
-static pid_t all_pids[10];
+static pid_t *all_pids;
 static int num_pids;
+static int max_pids;
+
+#define MIN_PIDS 16
 
 /** Fork and record the pid of the child. **/
 pid_t do_fork(void)
 {
 	pid_t newpid = fork();
 	
-	if (newpid != 0  &&  newpid != -1) {
+	/* we must ensure that newpid == -1 is not recorded in all_pids
+	   if fork() fails, otherwise kill_all() could nuke the world! */
+	if (newpid > 0) {
+		if (num_pids >= max_pids) {
+			max_pids = (max_pids ? 2*max_pids : MIN_PIDS);
+			all_pids = (pid_t *)Realloc(all_pids, sizeof(pid_t)*max_pids);
+			if (!all_pids) out_of_memory("do_fork");
+		}
 		all_pids[num_pids++] = newpid;
 	}
 	return newpid;
--- main.c.orig	Thu May  9 22:12:31 2002
+++ main.c	Thu May  9 22:15:28 2002
@@ -408,6 +408,13 @@
 			;
 	}
 
+	/* if fork fails, we must avoid doing kill(-1, SIGUSR2) later,
+	   since that could nuke the world! */
+	if (pid < 0) {
+		rprintf(FERROR,"could not fork in do_recv: %s\n",strerror(errno));
+		exit_cleanup(RERR_IPC);
+	}
+
 	close(recv_pipe[1]);
 	close(error_pipe[1]);
 	if (f_in != f_out) close(f_in);





More information about the rsync mailing list