fix subtle possible race that could kill another process and simplify code

Fri Aug 23 01:09:36 2002  George Lebl <jirka@5z.com>

	* gui/gdmlogin.c: fix subtle possible race that could kill another
	  process and simplify code in the meantime.  Ignore SIGCHLD, and
	  don't wait for the background process until we want to kill it,
	  that may leave a zombie for a little bit if the process dies early,
	  but who cares.

	* daemon/display.c, daemon/gdm.c, daemon/misc.c, daemon/server.c,
	  daemon/slave.c: Fix many many very minor races but such that
	  could conceivably take things down (a kill (0,...) "could"
	  very theoretically occur previously).  Fix some worse but still
	  not very likely races by using the push/pop of sigchld block in
	  places where we blocked by hand.  Ensure all processes are always
	  killed on slave exit (probably doing things redundantly in some
	  cases).  Fix some possible cases of syslog from a signal which
	  does bad things to our karma sometimes.  When the server dies
	  make sure we don't do anything with the display (we null the d->dsp
	  var) to make sure we don't reenter the xioerror_handler.  The
	  extra_process uses 0 and not -1 as the no value everywhere.
	  Make sure kills are never called with -1 (probably just anal,
	  but one never knows).  Do setsid and the init in daemonize
	  on -nodaemon as well.  Do setsid for greeter/chooser/config.

7 files changed, 183 insertions, 127 deletions

diff --git a/ChangeLog b/ChangeLog
index b053802e..ec9320b0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,27 @@
+Fri Aug 23 01:09:36 2002  George Lebl <jirka@5z.com>
+
+	* gui/gdmlogin.c: fix subtle possible race that could kill another
+	  process and simplify code in the meantime.  Ignore SIGCHLD, and
+	  don't wait for the background process until we want to kill it,
+	  that may leave a zombie for a little bit if the process dies early,
+	  but who cares.
+
+	* daemon/display.c, daemon/gdm.c, daemon/misc.c, daemon/server.c,
+	  daemon/slave.c: Fix many many very minor races but such that
+	  could conceivably take things down (a kill (0,...) "could"
+	  very theoretically occur previously).  Fix some worse but still
+	  not very likely races by using the push/pop of sigchld block in
+	  places where we blocked by hand.  Ensure all processes are always
+	  killed on slave exit (probably doing things redundantly in some
+	  cases).  Fix some possible cases of syslog from a signal which
+	  does bad things to our karma sometimes.  When the server dies
+	  make sure we don't do anything with the display (we null the d->dsp
+	  var) to make sure we don't reenter the xioerror_handler.  The
+	  extra_process uses 0 and not -1 as the no value everywhere.
+	  Make sure kills are never called with -1 (probably just anal,
+	  but one never knows).  Do setsid and the init in daemonize
+	  on -nodaemon as well.  Do setsid for greeter/chooser/config.
+
 Thu Aug 22 16:04:56 2002  George Lebl <jirka@5z.com>
 
 	* gui/gdmlanguages.c, config/locale.alias: The locale in the
diff --git a/daemon/display.c b/daemon/display.c
index 57647abe..c21eb0f8 100644
--- a/daemon/display.c
+++ b/daemon/display.c
@@ -169,11 +169,10 @@ gdm_display_manage (GdmDisplay *d)
     gdm_sigchld_block_push ();
 
     /* If we have an old slave process hanging around, kill it */
-    if (d->slavepid) {
-	    if (kill (d->slavepid, SIGINT) == 0)
+    if (d->slavepid > 0 &&
+	kill (d->slavepid, SIGINT) == 0)
 		    ve_waitpid_no_signal (d->slavepid, 0, 0);
-	    d->slavepid = 0;
-    }
+    d->slavepid = 0;
 
     /* Fork slave process */
     gdm_sigterm_block_push ();
@@ -279,11 +278,12 @@ gdm_display_unmanage (GdmDisplay *d)
 	       d->name, (int)d->slavepid);
 
     /* Kill slave */
-    if (d->slavepid != 0) {
-	if (kill (d->slavepid, SIGTERM) == 0)
-		ve_waitpid_no_signal (d->slavepid, 0, 0);
-	d->slavepid = 0;
-    }
+    gdm_sigchld_block_push ();
+    if (d->slavepid > 0 &&
+	kill (d->slavepid, SIGTERM) == 0)
+	    ve_waitpid_no_signal (d->slavepid, 0, 0);
+    d->slavepid = 0;
+    gdm_sigchld_block_pop ();
     
     if (d->type == TYPE_LOCAL)
 	d->dispstat = DISPLAY_DEAD;
diff --git a/daemon/gdm.c b/daemon/gdm.c
index 689243df..a8a4cca1 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -72,7 +72,7 @@ gint sessions = 0;		/* Number of remote sessions */
 gint flexi_servers = 0;		/* Number of flexi servers */
 uid_t GdmUserId;		/* Userid under which gdm should run */
 gid_t GdmGroupId;		/* Groupid under which gdm should run */
-pid_t extra_process = -1;	/* An extra process.  Used for quickie 
+pid_t extra_process = 0;	/* An extra process.  Used for quickie 
 				   processes, so that they also get whacked */
 int extra_status = 0;		/* Last status from the last extra process */
 pid_t gdm_main_pid = 0;
@@ -795,21 +795,20 @@ void
 gdm_final_cleanup (void)
 {
 	GSList *list, *li;
-	sigset_t mask;
 
 	gdm_debug ("gdm_final_cleanup");
 
-	sigemptyset (&mask);
-	sigaddset (&mask, SIGCHLD);
-	sigprocmask (SIG_BLOCK, &mask, NULL); 
+	gdm_sigchld_block_push ();
 
 	if (extra_process > 1) {
 		/* we sigterm extra processes, and we
 		 * don't wait */
 		kill (extra_process, SIGTERM);
-		extra_process = -1;
+		extra_process = 0;
 	}
 
+	gdm_sigchld_block_pop ();
+
 	list = g_slist_copy (displays);
 	for (li = list; li != NULL; li = li->next) {
 		GdmDisplay *d = li->data;
@@ -1007,7 +1006,7 @@ gdm_cleanup_children (void)
 
     if (pid == extra_process) {
 	    /* an extra process died, yay! */
-	    extra_process = -1;
+	    extra_process = 0;
 	    extra_status = exitstatus;
 	    return TRUE;
     }
@@ -1488,6 +1487,12 @@ main (int argc, char *argv[])
 	    fprintf (pf, "%d\n", (int)getpid());
 	    fclose (pf);
 	}
+
+	/* ignore failing, it MAY fail if it is already the leader */
+	setsid ();
+
+	chdir (GdmServAuthDir);
+	umask (022);
     }
     else
 	gdm_daemonify();
@@ -1646,9 +1651,11 @@ send_slave_ack (GdmDisplay *d)
 		not[1] = '\n';
 		write (d->master_notify_fd, not, 2);
 	}
+	gdm_sigchld_block_push ();
 	if (d->slavepid > 0) {
 		kill (d->slavepid, SIGUSR2);
 	}
+	gdm_sigchld_block_pop ();
 }
 
 static void
@@ -1661,9 +1668,11 @@ send_slave_command (GdmDisplay *d, const char *command)
 		write (d->master_notify_fd, cmd, strlen (cmd));
 		g_free (cmd);
 	}
+	gdm_sigchld_block_push ();
 	if (d->slavepid > 0) {
 		kill (d->slavepid, SIGUSR2);
 	}
+	gdm_sigchld_block_pop ();
 }
 
 
@@ -1996,8 +2005,10 @@ gdm_handle_message (GdmConnection *conn, const char *msg, gpointer data)
 		GSList *li;
 		for (li = displays; li != NULL; li = li->next) {
 			GdmDisplay *d = li->data;
+			gdm_sigchld_block_push ();
 			if (d->greetpid > 0)
 				kill (d->greetpid, SIGHUP);
+			gdm_sigchld_block_pop ();
 		}
 	} else if (strncmp (msg, GDM_SOP_WRITE_X_SERVERS " ",
 		            strlen (GDM_SOP_WRITE_X_SERVERS " ")) == 0) {
@@ -2375,8 +2386,10 @@ notify_displays_int (const char *key, int val)
 			gdm_fdprintf (disp->master_notify_fd,
 				      "%c%s %d\n",
 				      GDM_SLAVE_NOTIFY_KEY, key, val);
+			gdm_sigchld_block_push ();
 			if (disp->slavepid > 0)
 				kill (disp->slavepid, SIGUSR2);
+			gdm_sigchld_block_pop ();
 		}
 	}
 }
@@ -2391,8 +2404,10 @@ notify_displays_string (const char *key, const char *val)
 			gdm_fdprintf (disp->master_notify_fd,
 				      "%c%s %s\n",
 				      GDM_SLAVE_NOTIFY_KEY, key, val);
+			gdm_sigchld_block_push ();
 			if (disp->slavepid > 0)
 				kill (disp->slavepid, SIGUSR2);
+			gdm_sigchld_block_pop ();
 		}
 	}
 }
diff --git a/daemon/misc.c b/daemon/misc.c
index 26636bd1..048f5947 100644
--- a/daemon/misc.c
+++ b/daemon/misc.c
@@ -106,12 +106,14 @@ gdm_fail (const gchar *format, ...)
     } else if ( ! gdm_slave_final_cleanup ()) {
 	    /* If we weren't even a slave do some random cleanup only */
 	    /* FIXME: is this all fine? */
+	    gdm_sigchld_block_push ();
 	    if (extra_process > 1 && extra_process != getpid ()) {
 		    /* we sigterm extra processes, and we
 		     * don't wait */
 		    kill (extra_process, SIGTERM);
-		    extra_process = -1;
+		    extra_process = 0;
 	    }
+	    gdm_sigchld_block_pop ();
     }
 
     closelog ();
@@ -625,6 +627,8 @@ gdm_fork_extra (void)
 
 	gdm_sigterm_block_push ();
 	pid = extra_process = fork ();
+	if (pid < 0)
+		extra_process = 0;
 	gdm_sigterm_block_pop ();
 
 	gdm_sigchld_block_pop ();
@@ -640,7 +644,7 @@ gdm_wait_for_extra (int *status)
 	if (extra_process > 0) {
 		ve_waitpid_no_signal (extra_process, &extra_status, 0);
 	}
-	extra_process = -1;
+	extra_process = 0;
 
 	if (status != NULL)
 		*status = extra_status;
diff --git a/daemon/server.c b/daemon/server.c
index b608392a..f6bc89c3 100644
--- a/daemon/server.c
+++ b/daemon/server.c
@@ -113,7 +113,10 @@ gdm_server_reinit (GdmDisplay *disp)
 
 	gdm_debug ("gdm_server_reinit: Server for %s is about to be reinitialized!", disp->name);
 
-	kill (disp->servpid, SIGHUP);
+	gdm_sigchld_block_push ();
+	if (disp->servpid > 0)
+		kill (disp->servpid, SIGHUP);
+	gdm_sigchld_block_pop ();
 
 	/* HACK! the Xserver can't really tell us when it got the hup signal,
 	 * so we are really stuck just going to sleep for a bit hoping that
@@ -150,7 +153,7 @@ gdm_server_stop (GdmDisplay *disp)
 
     disp->servstat = SERVER_DEAD;
 
-    if (disp->servpid != 0) {
+    if (disp->servpid > 0) {
 	    pid_t servpid;
 
 	    gdm_debug ("gdm_server_stop: Killing server pid %d",
@@ -537,6 +540,8 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
     if (d->servpid > 0) {
 	    pid_t pid;
 
+	    d->dsp = NULL;
+
 	    gdm_sigchld_block_push ();
 	    pid = d->servpid;
 	    d->servpid = 0;
@@ -1048,7 +1053,7 @@ gdm_server_child_handler (int signal)
 			write (server_signal_pipe[1], "Yay!", 4);
 		} else if (pid == extra_process) {
 			/* an extra process died, yay! */
-			extra_process = -1;
+			extra_process = 0;
 			extra_status = status;
 		}
 	}
diff --git a/daemon/slave.c b/daemon/slave.c
index e0ff3f36..4fc19c65 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -149,6 +149,7 @@ static void     gdm_slave_alrm_handler (int sig);
 static void     gdm_slave_term_handler (int sig);
 static void     gdm_slave_child_handler (int sig);
 static void     gdm_slave_usr2_handler (int sig);
+static void     gdm_slave_quick_exit (gint status);
 static void     gdm_slave_exit (gint status, const gchar *format, ...) G_GNUC_PRINTF (2, 3);
 static void     gdm_child_exit (gint status, const gchar *format, ...) G_GNUC_PRINTF (2, 3);
 static gint     gdm_slave_exec_script (GdmDisplay *d, const gchar *dir,
@@ -515,7 +516,7 @@ gdm_slave_run (GdmDisplay *display)
 			       "In the meantime this display will be\n"
 			       "disabled.  Please restart gdm when\n"
 			       "the problem is corrected."));
-		    _exit (DISPLAY_ABORT);
+		    gdm_slave_quick_exit (DISPLAY_ABORT);
 	    }
 	    gdm_slave_send_num (GDM_SOP_XPID, d->servpid);
 
@@ -598,9 +599,9 @@ gdm_slave_run (GdmDisplay *display)
     if (d->handled && d->dsp == NULL) {
 	    gdm_server_stop (d);
 	    if (d->type == TYPE_LOCAL)
-		    _exit (DISPLAY_XFAILED);
+		    gdm_slave_quick_exit (DISPLAY_XFAILED);
 	    else
-		    _exit (DISPLAY_ABORT);
+		    gdm_slave_quick_exit (DISPLAY_ABORT);
     }
 
     /* Some sort of a bug foo to make some servers work or whatnot,
@@ -634,7 +635,7 @@ gdm_slave_run (GdmDisplay *display)
 	    /* yay, we now wait for the server to die,
 	     * which will in fact just exit, so
 	     * this code is a little bit too anal */
-	    while (d->servpid != 0) {
+	    while (d->servpid > 0) {
 		    select (0, NULL, NULL, NULL, NULL);
 	    }
 	    return;
@@ -663,9 +664,7 @@ gdm_slave_run (GdmDisplay *display)
 	    gdm_debug ("gdm_slave_run: Automatic login done");
 	    
 	    if (remanage_asap) {
-		    gdm_server_stop (d);
-		    gdm_verify_cleanup (d);
-		    _exit (DISPLAY_REMANAGE);
+		    gdm_slave_quick_exit (DISPLAY_REMANAGE);
 	    }
 
 	    /* return to gdm_slave_start so that the server
@@ -708,9 +707,7 @@ gdm_slave_run (GdmDisplay *display)
 	    gdm_slave_send_string (GDM_SOP_LOGIN, "");
 
 	    if (remanage_asap) {
-		    gdm_server_stop (d);
-		    gdm_verify_cleanup (d);
-		    _exit (DISPLAY_REMANAGE);
+		    gdm_slave_quick_exit (DISPLAY_REMANAGE);
 	    }
 
 	    if (greet) {
@@ -833,15 +830,18 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 
 	if (pid < 0) {
 		/* return left pointer */
-		Cursor xcursor = XCreateFontCursor (d->dsp, GDK_LEFT_PTR);
+		Cursor xcursor;
+
+		/* can't fork, damnit */
+		display->sesspid = 0;
+	       
+		xcursor = XCreateFontCursor (d->dsp, GDK_LEFT_PTR);
 		XDefineCursor (d->dsp,
 			       DefaultRootWindow (d->dsp),
 			       xcursor);
 		XFreeCursor (d->dsp, xcursor);
 		XSync (d->dsp, False);
 
-		/* can't fork, damnit */
-		display->sesspid = 0;
 		return;
 	}
 
@@ -849,6 +849,8 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 		char **argv;
 		/* child */
 
+		setsid ();
+
 		setuid (0);
 		setgid (0);
 
@@ -899,8 +901,8 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 				 "default location."));
 
 		argv = ve_split
-			(EXPANDED_GDMCONFIGDIR
-			 "/gdmconfig --disable-sound --disable-crash-dialog");
+			(EXPANDED_BINDIR
+			 "/gdmsetup --disable-sound --disable-crash-dialog");
 		if (access (argv[0], X_OK) == 0)
 			execv (argv[0], argv);
 
@@ -1111,9 +1113,7 @@ gdm_slave_wait_for_login (void)
 			d->logged_in = FALSE;
 
 			if (remanage_asap) {
-				gdm_server_stop (d);
-				gdm_verify_cleanup (d);
-				_exit (DISPLAY_REMANAGE);
+				gdm_slave_quick_exit (DISPLAY_REMANAGE);
 			}
 
 			greeter_no_focus = FALSE;
@@ -1455,6 +1455,8 @@ gdm_slave_greeter (void)
     switch (pid) {
 	
     case 0:
+	setsid ();
+
 	sigfillset (&mask);
 	sigdelset (&mask, SIGINT);
 	sigdelset (&mask, SIGTERM);
@@ -1612,6 +1614,7 @@ gdm_slave_greeter (void)
 	gdm_child_exit (DISPLAY_ABORT, _("%s: Error starting greeter on display %s"), "gdm_slave_greeter", d->name);
 	
     case -1:
+	d->greetpid = 0;
 	gdm_slave_exit (DISPLAY_ABORT, _("%s: Can't fork gdmgreeter process"), "gdm_slave_greeter");
 	
     default:
@@ -1813,6 +1816,8 @@ gdm_slave_chooser (void)
 	switch (pid) {
 
 	case 0:
+		setsid ();
+
 		sigfillset (&mask);
 		sigdelset (&mask, SIGINT);
 		sigdelset (&mask, SIGTERM);
@@ -1923,7 +1928,7 @@ gdm_slave_chooser (void)
 				buf[bytes] ='\0';
 			send_chosen_host (d, buf);
 
-			_exit (DISPLAY_CHOSEN);
+			gdm_slave_quick_exit (DISPLAY_CHOSEN);
 		}
 
 		close (p[0]);
@@ -2816,10 +2821,7 @@ gdm_slave_session_start (void)
 	    gdm_slave_session_stop (0);
 	    gdm_slave_session_cleanup ();
 
-	    gdm_server_stop (d);
-	    gdm_verify_cleanup (d);
-
-	    _exit (DISPLAY_REMANAGE);
+	    gdm_slave_quick_exit (DISPLAY_REMANAGE);
     }
 
     if (strcmp (session, GDM_SESSION_FAILSAFE_GNOME) == 0 ||
@@ -2958,7 +2960,7 @@ gdm_slave_session_stop (pid_t sesspid)
 				  "/", d->name, ".Xservers", NULL);
 
     /* if there was a session that ran, run the PostSession script */
-    if (sesspid != 0) {
+    if (sesspid > 0) {
 	    /* setup some env for PostSession script */
 	    gnome_setenv ("DISPLAY", d->name, TRUE);
 	    gnome_setenv ("XAUTHORITY", d->authfile, TRUE);
@@ -3020,8 +3022,6 @@ gdm_slave_session_cleanup (void)
 static void
 gdm_slave_term_handler (int sig)
 {
-	sigset_t tmask;
-
 	gdm_in_signal++;
 
 	gdm_debug ("gdm_slave_term_handler: %s got TERM/INT signal", d->name);
@@ -3030,43 +3030,46 @@ gdm_slave_term_handler (int sig)
 	seteuid (0);
 	setegid (0);
 
-	sigemptyset (&tmask);
-	sigaddset (&tmask, SIGCHLD);
-	sigprocmask (SIG_BLOCK, &tmask, NULL);  
+	gdm_sigchld_block_push ();
 
 	if (extra_process > 1) {
 		/* we sigterm extra processes, and we
 		 * don't wait */
 		kill (extra_process, SIGTERM);
-		extra_process = -1;
+		extra_process = 0;
 	}
 
-	if (d->greetpid != 0) {
+	gdm_sigchld_block_pop ();
+
+	if (d->greetpid > 0) {
 		pid_t pid = d->greetpid;
+		gdm_sigchld_block_push ();
 		d->greetpid = 0;
 		greet = FALSE;
 		gdm_debug ("gdm_slave_term_handler: Whacking greeter");
-		if (kill (pid, sig) == 0)
+		if (pid > 0 &&
+		    kill (pid, sig) == 0)
 			ve_waitpid_no_signal (pid, 0, 0); 
+		gdm_sigchld_block_pop ();
 	} else if (login != NULL) {
 		gdm_slave_session_stop (d->sesspid);
 		gdm_slave_session_cleanup ();
 	}
 
-	if (d->chooserpid != 0) {
+	if (d->chooserpid > 0) {
 		pid_t pid = d->chooserpid;
+		gdm_sigchld_block_push ();
 		d->chooserpid = 0;
 		gdm_debug ("gdm_slave_term_handler: Whacking chooser");
-		if (kill (pid, sig) == 0)
+		if (pid > 0 &&
+		    kill (pid, sig) == 0)
 			ve_waitpid_no_signal (pid, 0, 0); 
+		gdm_sigchld_block_pop ();
 	}
 
 	gdm_debug ("gdm_slave_term_handler: Whacking server");
 
-	gdm_server_stop (d);
-	gdm_verify_cleanup (d);
-
-	_exit (DISPLAY_ABORT);
+	gdm_slave_quick_exit (DISPLAY_ABORT);
 }
 
 /* called on alarms to ping */
@@ -3153,6 +3156,12 @@ gdm_slave_child_handler (int sig)
 			return;
 		}
 
+		/* Well now we're just going to kill
+		 * everything including the X server,
+		 * so no need doing XCloseDisplay which
+		 * may just get us an XIOError */
+		d->dsp = NULL;
+
 		whack_greeter_fds ();
 
 		/* just for paranoia's sake */
@@ -3172,9 +3181,9 @@ gdm_slave_child_handler (int sig)
 		     WEXITSTATUS (status) == DISPLAY_HALT ||
 		     WEXITSTATUS (status) == DISPLAY_SUSPEND ||
 		     WEXITSTATUS (status) == DISPLAY_RESTARTGDM)) {
-			_exit (WEXITSTATUS (status));
+			gdm_slave_quick_exit (WEXITSTATUS (status));
 		} else {
-			_exit (DISPLAY_REMANAGE);
+			gdm_slave_quick_exit (DISPLAY_REMANAGE);
 		}
 	} else if (pid != 0 && pid == d->sesspid) {
 		d->sesspid = 0;
@@ -3184,12 +3193,19 @@ gdm_slave_child_handler (int sig)
 		d->servstat = SERVER_DEAD;
 		d->servpid = 0;
 		gdm_server_wipe_cookies (d);
+		gdm_slave_whack_temp_auth_file ();
+
+		/* whack the session good */
+		if (d->sesspid > 0)
+			kill (- (d->sesspid), SIGTERM);
 
+		/* if not handled there is no need for further formalities,
+		 * we just have to die */
 		if ( ! d->handled)
-			_exit (DISPLAY_REMANAGE);
+			gdm_slave_quick_exit (DISPLAY_REMANAGE);
 	} else if (pid == extra_process) {
 		/* an extra process died, yay! */
-		extra_process = -1;
+		extra_process = 0;
 	    	extra_status = status;
 	}
     }
@@ -3250,9 +3266,7 @@ gdm_slave_usr2_handler (int sig)
 				if (d->type != TYPE_FLEXI_XNEST &&
 				    d->type != TYPE_FLEXI) {
 					if ( ! d->logged_in) {
-						gdm_server_stop (d);
-						gdm_verify_cleanup (d);
-						_exit (DISPLAY_REMANAGE);
+						gdm_slave_quick_exit (DISPLAY_REMANAGE);
 					} else {
 						remanage_asap = TRUE;
 					}
@@ -3278,7 +3292,8 @@ gdm_slave_xerror_handler (Display *disp, XErrorEvent *evt)
 static gint
 gdm_slave_xioerror_handler (Display *disp)
 {
-	sigset_t tmask;
+	/* Display is all gone */
+	d->dsp = NULL;
 
 	gdm_debug ("gdm_slave_xioerror_handler: I/O error for display %s", d->name);
 
@@ -3286,26 +3301,28 @@ gdm_slave_xioerror_handler (Display *disp)
 	seteuid (0);
 	setegid (0);
 
-	sigemptyset (&tmask);
-	sigaddset (&tmask, SIGCHLD);
-	sigprocmask (SIG_BLOCK, &tmask, NULL);  
-
-	if (d->greetpid != 0) {
+	if (d->greetpid > 0) {
 		pid_t pid = d->greetpid;
+		gdm_sigchld_block_push ();
 		d->greetpid = 0;
 		greet = FALSE;
-		if (kill (pid, SIGINT) == 0)
+		if (pid > 0 &&
+		    kill (pid, SIGINT) == 0)
 			ve_waitpid_no_signal (pid, 0, 0); 
+		gdm_sigchld_block_pop ();
 	} else if (login != NULL) {
 		gdm_slave_session_stop (d->sesspid);
 		gdm_slave_session_cleanup ();
 	}
 
-	if (d->chooserpid != 0) {
+	if (d->chooserpid > 0) {
 		pid_t pid = d->chooserpid;
+		gdm_sigchld_block_push ();
 		d->chooserpid = 0;
-		if (kill (pid, SIGINT) == 0)
+		if (pid > 0 &&
+		    kill (pid, SIGINT) == 0)
 			ve_waitpid_no_signal (pid, 0, 0); 
+		gdm_sigchld_block_pop ();
 	}
     
 	gdm_error (_("gdm_slave_xioerror_handler: Fatal X error - Restarting %s"), d->name);
@@ -3317,10 +3334,12 @@ gdm_slave_xioerror_handler (Display *disp)
 	     d->type == TYPE_FLEXI) &&
 	    (do_xfailed_on_xio_error ||
 	     d->starttime + 5 >= time (NULL))) {
-		_exit (DISPLAY_XFAILED);
+		gdm_slave_quick_exit (DISPLAY_XFAILED);
 	} else {
-		_exit (DISPLAY_REMANAGE);
+		gdm_slave_quick_exit (DISPLAY_REMANAGE);
 	}
+
+	return 0;
 }
 
 static void
@@ -3422,57 +3441,69 @@ gdm_slave_greeter_ctl_no_ret (char cmd, const char *str)
 	g_free (gdm_slave_greeter_ctl (cmd, str));
 }
 
-
 static void 
-gdm_slave_exit (gint status, const gchar *format, ...)
+gdm_slave_quick_exit (gint status)
 {
-    va_list args;
-    gchar *s;
-
-    va_start (args, format);
-    s = g_strdup_vprintf (format, args);
-    va_end (args);
-    
-    syslog (LOG_ERR, "%s", s);
-    
-    g_free (s);
-
     /* just for paranoia's sake */
     seteuid (0);
     setegid (0);
 
     if (d != NULL) {
-	    sigset_t tmask;
+	    /* Well now we're just going to kill
+	     * everything including the X server,
+	     * so no need doing XCloseDisplay which
+	     * may just get us an XIOError */
+	    d->dsp = NULL;
 
-	    sigemptyset (&tmask);
-	    sigaddset (&tmask, SIGCHLD);
-	    sigprocmask (SIG_BLOCK, &tmask, NULL);  
+	    /* Push and never pop */
+	    gdm_sigchld_block_push ();
 
 	    /* Kill children where applicable */
-	    if (d->greetpid != 0)
+	    if (d->greetpid > 0)
 		    kill (d->greetpid, SIGTERM);
 	    d->greetpid = 0;
 
-	    if (d->chooserpid != 0)
+	    if (d->chooserpid > 0)
 		    kill (d->chooserpid, SIGTERM);
 	    d->chooserpid = 0;
 
-	    if (d->sesspid != 0)
+	    if (d->sesspid > 0)
 		    kill (-(d->sesspid), SIGTERM);
 	    d->sesspid = 0;
 
 	    gdm_server_stop (d);
 	    gdm_verify_cleanup (d);
 
-	    if (d->servpid != 0)
+	    if (d->servpid > 0)
 		    kill (d->servpid, SIGTERM);
 	    d->servpid = 0;
+
+	    if (extra_process > 0)
+		    kill (extra_process, SIGTERM);
+	    extra_process = 0;
     }
 
     _exit (status);
 }
 
 static void 
+gdm_slave_exit (gint status, const gchar *format, ...)
+{
+    va_list args;
+    gchar *s;
+
+    va_start (args, format);
+    s = g_strdup_vprintf (format, args);
+    va_end (args);
+    
+    gdm_error ("%s", s);
+    
+    g_free (s);
+
+    gdm_slave_quick_exit (status);
+}
+
+static void 
 gdm_child_exit (gint status, const gchar *format, ...)
 {
     va_list args;
@@ -3855,9 +3886,7 @@ gdm_slave_handle_notify (const char *msg)
 				/* FIXME: can't handle flexi servers like this
 				 * without going all cranky */
 				if ( ! d->logged_in) {
-					gdm_server_stop (d);
-					gdm_verify_cleanup (d);
-					_exit (DISPLAY_REMANAGE);
+					gdm_slave_quick_exit (DISPLAY_REMANAGE);
 				} else {
 					remanage_asap = TRUE;
 				}
@@ -3875,9 +3904,7 @@ gdm_slave_handle_notify (const char *msg)
 				/* FIXME: can't handle flexi servers like this
 				 * without going all cranky */
 				if ( ! d->logged_in) {
-					gdm_server_stop (d);
-					gdm_verify_cleanup (d);
-					_exit (DISPLAY_REMANAGE);
+					gdm_slave_quick_exit (DISPLAY_REMANAGE);
 				} else {
 					remanage_asap = TRUE;
 				}
@@ -3889,9 +3916,7 @@ gdm_slave_handle_notify (const char *msg)
 		/* FIXME: can't handle flexi servers without going all cranky */
 		if (d->type == TYPE_LOCAL || d->type == TYPE_XDMCP) {
 			if ( ! d->logged_in) {
-				gdm_server_stop (d);
-				gdm_verify_cleanup (d);
-				_exit (DISPLAY_REMANAGE);
+				gdm_slave_quick_exit (DISPLAY_REMANAGE);
 			} else {
 				remanage_asap = TRUE;
 			}
diff --git a/gui/gdmlogin.c b/gui/gdmlogin.c
index c3939dac..d120f2d4 100644
--- a/gui/gdmlogin.c
+++ b/gui/gdmlogin.c
@@ -281,21 +281,12 @@ setup_cursor (GdkCursorType type)
 }
 
 static void
-gdm_greeter_chld (int sig)
-{
-	if (backgroundpid != 0 &&
-	    waitpid (backgroundpid, NULL, WNOHANG) > 0) {
-		backgroundpid = 0;
-	}
-}
-
-static void
 kill_thingies (void)
 {
 	pid_t pid = backgroundpid;
 
 	backgroundpid = 0;
-	if (pid != 0) {
+	if (pid > 0) {
 		if (kill (pid, SIGTERM) == 0)
 			waitpid (pid, NULL, 0);
 	}
@@ -4022,7 +4013,6 @@ main (int argc, char *argv[])
 {
     struct sigaction hup;
     struct sigaction term;
-    struct sigaction chld;
     sigset_t mask;
     GIOChannel *ctrlch;
     const char *gdm_version;
@@ -4212,19 +4202,12 @@ main (int argc, char *argv[])
     if (sigaction (SIGTERM, &term, NULL) < 0) 
         gdm_login_abort (_("main: Error setting up TERM signal handler"));
 
-    chld.sa_handler = gdm_greeter_chld;
-    chld.sa_flags = SA_RESTART;
-    sigemptyset(&chld.sa_mask);
-    sigaddset (&chld.sa_mask, SIGCHLD);
-
-    if (sigaction (SIGCHLD, &chld, NULL) < 0) 
-        gdm_login_abort (_("main: Error setting up CHLD signal handler"));
-
     sigfillset (&mask);
     sigdelset (&mask, SIGTERM);
     sigdelset (&mask, SIGHUP);
     sigdelset (&mask, SIGINT);
-    sigdelset (&mask, SIGCHLD);
+    /* ignore SIGCHLD */
+    sigaddset (&mask, SIGCHLD);
     
     if (sigprocmask (SIG_SETMASK, &mask, NULL) == -1) 
 	gdm_login_abort (_("Could not set signal mask!"));