Super fork race killing patch, suspend both SIGCHLD and TERM/INT/HUP over

Thu Oct 25 20:28:03 2001  George Lebl <jirka@5z.com>

	* daemon/misc.[ch], daemon/display.c, daemon/errorgui.c,
	  daemon/gdm.c, daemon/server.c, daemon/slave.c:  Super fork race
	  killing patch, suspend both SIGCHLD and TERM/INT/HUP over
	  forks, suspend CHLD over short waitpids.  Fix races where
	  the switch logic after forks may have gotten confused.  A tiny bit
	  of unrelated cleanup as well.

8 files changed, 483 insertions, 333 deletions

diff --git a/ChangeLog b/ChangeLog
index af7a546e..177340ac 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+Thu Oct 25 20:28:03 2001  George Lebl <jirka@5z.com>
+
+	* daemon/misc.[ch], daemon/display.c, daemon/errorgui.c,
+	  daemon/gdm.c, daemon/server.c, daemon/slave.c:  Super fork race
+	  killing patch, suspend both SIGCHLD and TERM/INT/HUP over 
+	  forks, suspend CHLD over short waitpids.  Fix races where
+	  the switch logic after forks may have gotten confused.  A tiny bit
+	  of unrelated cleanup as well.
+
 Thu Oct 25 01:49:57 2001  George Lebl <jirka@5z.com>
 
 	* config/XKeepsCrashing: fix minor issues
diff --git a/daemon/display.c b/daemon/display.c
index 885f4ecf..9a39e3d9 100644
--- a/daemon/display.c
+++ b/daemon/display.c
@@ -132,7 +132,7 @@ gdm_display_check_loop (GdmDisplay *disp)
 gboolean 
 gdm_display_manage (GdmDisplay *d)
 {
-    sigset_t mask, omask;
+    pid_t pid;
     int i;
 
     if (!d) 
@@ -143,22 +143,23 @@ gdm_display_manage (GdmDisplay *d)
     if ( ! gdm_display_check_loop (d))
 	    return FALSE;
 
+    gdm_sigchld_block_push ();
+
     /* If we have an old slave process hanging around, kill it */
     if (d->slavepid) {
-	sigemptyset (&mask);
-	sigaddset (&mask, SIGCHLD);
-	sigprocmask (SIG_BLOCK, &mask, &omask); 
-
-	if (kill (d->slavepid, SIGINT) == 0)
-		waitpid (d->slavepid, 0, 0);
-	d->slavepid = 0;
-
-	sigprocmask (SIG_SETMASK, &omask, NULL);
+	    if (kill (d->slavepid, SIGINT) == 0)
+		    waitpid (d->slavepid, 0, 0);
+	    d->slavepid = 0;
     }
 
     /* Fork slave process */
-    gdm_safe_fork (&(d->slavepid));
-    switch (d->slavepid) {
+    gdm_sigterm_block_push ();
+    pid = d->slavepid = fork ();
+    gdm_sigterm_block_push ();
+
+    gdm_sigchld_block_pop ();
+
+    switch (pid) {
 
     case 0:
 	setpgid (0, 0);
@@ -213,7 +214,7 @@ gdm_display_manage (GdmDisplay *d)
 
     default:
 	gdm_debug ("gdm_display_manage: Forked slave: %d",
-		   (int)d->slavepid);
+		   (int)pid);
 	break;
     }
 
diff --git a/daemon/errorgui.c b/daemon/errorgui.c
index ce62ccba..08320d8a 100644
--- a/daemon/errorgui.c
+++ b/daemon/errorgui.c
@@ -35,7 +35,6 @@
 extern char **stored_argv;
 extern int stored_argc;
 extern char *stored_path;
-extern pid_t extra_process;
 
 static gboolean
 gdm_event (GtkObject *object,
@@ -129,8 +128,8 @@ gdm_error_box (GdmDisplay *d, const char *dialog_type, const char *error)
 {
 	pid_t pid;
 
-	gdm_safe_fork (&extra_process);
-	pid = extra_process;
+	pid = gdm_fork_extra ();
+
 	if (pid == 0) {
 		char *geom;
 		int i;
@@ -165,10 +164,7 @@ gdm_error_box (GdmDisplay *d, const char *dialog_type, const char *error)
 		gdm_error (_("gdm_error_box: Failed to execute self"));
 		_exit (1);
 	} else if (pid > 0) {
-		gdm_debug ("gdm_error_box: Ran error box, waiting...");
-		waitpid (pid, 0, 0);
-		gdm_debug ("gdm_error_box: Wait done");
-		extra_process = -1;
+		gdm_wait_for_extra (NULL);
 	} else {
 		gdm_error (_("gdm_error_box: Cannot fork to display error/info box"));
 	}
@@ -268,8 +264,7 @@ gdm_failsafe_question (GdmDisplay *d,
 	if (pipe (p) < 0)
 		return NULL;
 
-	gdm_safe_fork (&extra_process);
-	pid = extra_process;
+	pid = gdm_fork_extra ();
 	if (pid == 0) {
 		char *geom;
 		int i;
@@ -311,9 +306,11 @@ gdm_failsafe_question (GdmDisplay *d,
 	} else if (pid > 0) {
 		char buf[BUFSIZ];
 		int bytes;
+
 		close (p[1]);
-		waitpid (pid, 0, 0);
-		extra_process = -1;
+
+		gdm_wait_for_extra (NULL);
+
 		bytes = read (p[0], buf, BUFSIZ-1);
 		if (bytes > 0) {
 			close (p[0]);
diff --git a/daemon/gdm.c b/daemon/gdm.c
index 4900fb36..a1320fad 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -76,6 +76,7 @@ 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 
 				   processes, so that they also get whacked */
+int extra_status = 0;		/* Last status from the last extra process */
 
 GdmConnection *fifoconn = NULL; /* Fifo connection */
 GdmConnection *unixconn = NULL; /* UNIX Socket connection */
@@ -626,7 +627,6 @@ final_cleanup (void)
 {
 	GSList *list;
 	sigset_t mask;
-	gchar *path;
 
 	gdm_debug ("final_cleanup");
 
@@ -648,22 +648,26 @@ final_cleanup (void)
 	/* Close stuff */
 
 	gdm_xdmcp_close ();
-	gdm_connection_close (fifoconn);
-	fifoconn = NULL;
-	gdm_connection_close (unixconn);
-	unixconn = NULL;
-
-	/* Unlink the connections */
-
-	path = g_strconcat (GdmServAuthDir, "/.gdmfifo", NULL);
-	unlink (path);
-	g_free (path);
 
-	unlink (GDM_SUP_SOCKET);
+	if (fifoconn != NULL) {
+		char *path;
+		gdm_connection_close (fifoconn);
+		path = g_strconcat (GdmServAuthDir, "/.gdmfifo", NULL);
+		unlink (path);
+		g_free (path);
+		fifoconn = NULL;
+	}
 
+	if (unixconn != NULL) {
+		gdm_connection_close (unixconn);
+		unlink (GDM_SUP_SOCKET);
+		unixconn = NULL;
+	}
 
 	closelog();
-	unlink (GdmPidFile);
+
+	if (GdmPidFile != NULL)
+		unlink (GdmPidFile);
 }
 
 static gboolean
@@ -696,8 +700,7 @@ deal_with_x_crashes (GdmDisplay *d)
 	    gdm_info (_("deal_with_x_crashes: Running the "
 			"XKeepsCrashing script"));
 
-	    gdm_safe_fork (&extra_process);
-	    pid = extra_process;
+	    pid = gdm_fork_extra ();
 
 	    if (pid == 0) {
 		    char *argv[2];
@@ -736,9 +739,8 @@ deal_with_x_crashes (GdmDisplay *d)
 		    _exit (32);
 	    } else if (pid > 0) {
 		    int status;
-		    waitpid (pid, &status, 0);
 
-		    extra_process = -1;
+		    gdm_wait_for_extra (&status);
 
 		    if (WIFEXITED (status) &&
 			WEXITSTATUS (status) == 0) {
@@ -833,6 +835,7 @@ gdm_cleanup_children (void)
     if (pid == extra_process) {
 	    /* an extra process died, yay! */
 	    extra_process = -1;
+	    extra_status = exitstatus;
 	    return TRUE;
     }
 
diff --git a/daemon/misc.c b/daemon/misc.c
index 351938db..bbe1cc04 100644
--- a/daemon/misc.c
+++ b/daemon/misc.c
@@ -45,6 +45,7 @@ extern GSList *displays;
 extern char **environ;
 
 extern pid_t extra_process;
+extern int extra_status;
 
 
 /**
@@ -410,12 +411,7 @@ gdm_exec_wait (char * const *argv, gboolean no_display)
 	    access (argv[0], X_OK) != 0)
 		return -1;
 
-	/* Note a fun and unavoidable (also almost
-	 * impossible to happen race.  If the parent gets
-	 * whacked before it executes any code, it will
-	 * not whack the child.  Oh well. */
-	gdm_safe_fork (&extra_process);
-	pid = extra_process;
+	pid = gdm_fork_extra ();
 	if (pid == 0) {
 		int i;
 
@@ -441,9 +437,7 @@ gdm_exec_wait (char * const *argv, gboolean no_display)
 	if (pid < 0)
 		return -1;
 
-	waitpid (pid, &status, 0);
-
-	extra_process = -1;
+	gdm_wait_for_extra (&status);
 
 	if (WIFEXITED (status))
 		return WEXITSTATUS (status);
@@ -451,20 +445,91 @@ gdm_exec_wait (char * const *argv, gboolean no_display)
 		return -1;
 }
 
+static int sigchld_blocked = 0;
+static sigset_t sigchldblock_mask, sigchldblock_oldmask;
+
+static int sigterm_blocked = 0;
+static sigset_t sigtermblock_mask, sigtermblock_oldmask;
+
+void
+gdm_sigchld_block_push (void)
+{
+	sigchld_blocked ++;
+
+	if (sigchld_blocked == 1) {
+		/* Set signal mask */
+		sigemptyset (&sigchldblock_mask);
+		sigaddset (&sigchldblock_mask, SIGCHLD);
+		sigprocmask (SIG_BLOCK, &sigchldblock_mask, &sigchldblock_oldmask);
+	}
+}
+
+void
+gdm_sigchld_block_pop (void)
+{
+	sigchld_blocked --;
+
+	if (sigchld_blocked == 0) {
+		/* reset signal mask back */
+		sigprocmask (SIG_SETMASK, &sigchldblock_oldmask, NULL);
+	}
+}
+
+void
+gdm_sigterm_block_push (void)
+{
+	sigterm_blocked ++;
+
+	if (sigterm_blocked == 1) {
+		/* Set signal mask */
+		sigemptyset (&sigtermblock_mask);
+		sigaddset (&sigtermblock_mask, SIGTERM);
+		sigaddset (&sigtermblock_mask, SIGINT);
+		sigaddset (&sigtermblock_mask, SIGHUP);
+		sigprocmask (SIG_BLOCK, &sigtermblock_mask, &sigtermblock_oldmask);
+	}
+}
+
+void
+gdm_sigterm_block_pop (void)
+{
+	sigterm_blocked --;
+
+	if (sigterm_blocked == 0) {
+		/* reset signal mask back */
+		sigprocmask (SIG_SETMASK, &sigtermblock_oldmask, NULL);
+	}
+}
+
+pid_t
+gdm_fork_extra (void)
+{
+	pid_t pid;
+	gdm_sigchld_block_push ();
+
+	gdm_sigterm_block_push ();
+	pid = extra_process = fork ();
+	gdm_sigterm_block_pop ();
+
+	gdm_sigchld_block_pop ();
+
+	return pid;
+}
+
 void
-gdm_safe_fork (pid_t *pid)
+gdm_wait_for_extra (int *status)
 {
-    sigset_t mask, oldmask;
+	gdm_sigchld_block_push ();
 
-    /* Set signal mask */
-    sigemptyset (&mask);
-    sigaddset (&mask, SIGCHLD);
-    sigprocmask (SIG_BLOCK, &mask, &oldmask);
+	if (extra_process > 0) {
+		waitpid (extra_process, &extra_status, 0);
+	}
+	extra_process = -1;
 
-    *pid = fork ();
+	if (status != NULL)
+		*status = extra_status;
 
-    /* reset signal mask back */
-    sigprocmask (SIG_SETMASK, &oldmask, NULL);
+	gdm_sigchld_block_pop ();
 }
 
 /* done before each login.  This can do so sanity ensuring,
diff --git a/daemon/misc.h b/daemon/misc.h
index b01bed4e..2e70668b 100644
--- a/daemon/misc.h
+++ b/daemon/misc.h
@@ -42,10 +42,14 @@ int	gdm_exec_wait (char * const *argv, gboolean no_display);
  * exists and has the correct permissions */
 void	gdm_ensure_sanity	(void);
 
-/* This is a race free fork, that is the pid will
- * be set before sigchld is sent.  This is achieved by
- * blocking sigchld for the moment */
-void	gdm_safe_fork (pid_t *pid);
+/* This is for race free forks */
+void	gdm_sigchld_block_push (void);
+void	gdm_sigchld_block_pop (void);
+void	gdm_sigterm_block_push (void);
+void	gdm_sigterm_block_pop (void);
+
+pid_t	gdm_fork_extra (void);
+void	gdm_wait_for_extra (int *status);
 
 #endif /* GDM_MISC_H */
 
diff --git a/daemon/server.c b/daemon/server.c
index a729124b..7695fb80 100644
--- a/daemon/server.c
+++ b/daemon/server.c
@@ -64,6 +64,7 @@ extern gboolean GdmXdmcp;
 extern sigset_t sysmask;
 extern gint high_display_num;
 extern pid_t extra_process;
+extern int extra_status;
 
 /* Global vars */
 static GdmDisplay *d = NULL;
@@ -144,8 +145,12 @@ gdm_server_stop (GdmDisplay *disp)
 
 	    gdm_debug ("gdm_server_stop: Killing server pid %d", (int)servpid);
 
-	    if (kill (servpid, SIGTERM) == 0)
+	    gdm_sigchld_block_push ();
+	    if (servpid > 0 &&
+		kill (servpid, SIGTERM) == 0)
 		    waitpid (servpid, 0, 0);
+	    gdm_sigchld_block_pop ();
+
 
 	    gdm_debug ("gdm_server_stop: Server pid %d dead", (int)servpid);
     }
@@ -461,9 +466,13 @@ gdm_server_start (GdmDisplay *disp, gboolean treat_as_flexi,
 
     /* bad things are happening */
     if (d->servpid > 0) {
-	    if (kill (d->servpid, SIGTERM) == 0)
-		    waitpid (d->servpid, NULL, 0);
+	    pid_t pid = d->servpid;
 	    d->servpid = 0;
+	    gdm_sigchld_block_push ();
+	    if (pid > 0 &&
+		kill (pid, SIGTERM) == 0)
+		    waitpid (pid, NULL, 0);
+	    gdm_sigchld_block_pop ();
     }
 
     /* We will rebake cookies anyway, so wipe these */
@@ -563,6 +572,7 @@ gdm_server_spawn (GdmDisplay *d)
     int len, i;
     const char *command;
     char *bin;
+    pid_t pid;
 
     if (d == NULL ||
 	ve_string_empty (d->command)) {
@@ -571,18 +581,25 @@ gdm_server_spawn (GdmDisplay *d)
 
     d->servstat = SERVER_STARTED;
 
+    gdm_sigchld_block_push ();
+
     /* eek, some previous copy, just wipe it */
     if (d->servpid > 0) {
-	    if (kill (d->servpid, SIGTERM) == 0)
-		    waitpid (d->servpid, NULL, 0);
+	    pid_t pid = d->servpid;
+	    d->servpid = 0;
+	    if (kill (pid, SIGTERM) == 0)
+		    waitpid (pid, NULL, 0);
     }
     
     /* Fork into two processes. Parent remains the gdm process. Child
      * becomes the X server. */
 
-    gdm_safe_fork (&(d->servpid));
+    gdm_sigterm_block_push ();
+    pid = d->servpid = fork ();
+    gdm_sigterm_block_pop ();
+    gdm_sigchld_block_pop ();
     
-    switch (d->servpid) {
+    switch (pid) {
 	
     case 0:
         alarm (0);
@@ -785,7 +802,7 @@ gdm_server_spawn (GdmDisplay *d)
 	break;
 	
     default:
-	gdm_debug ("gdm_server_spawn: Forked server on pid %d", (int)d->servpid);
+	gdm_debug ("gdm_server_spawn: Forked server on pid %d", (int)pid);
 	break;
     }
 }
@@ -864,6 +881,7 @@ gdm_server_child_handler (int signal)
 		} else if (pid == extra_process) {
 			/* an extra process died, yay! */
 			extra_process = -1;
+			extra_status = status;
 		}
 	}
 }
diff --git a/daemon/slave.c b/daemon/slave.c
index 7ff5c2db..492c6b65 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -75,6 +75,7 @@ static gchar *ParsedTimedLogin = NULL;
 extern gboolean gdm_first_login;
 extern gboolean gdm_emergency_server;
 extern pid_t extra_process;
+extern int extra_status;
 
 /* Configuration option variables */
 extern gchar *GdmUser;
@@ -484,11 +485,7 @@ gdm_slave_run (GdmDisplay *display)
 static void
 gdm_slave_whack_greeter (void)
 {
-	sigset_t tmask, omask;
-
-	sigemptyset (&tmask);
-	sigaddset (&tmask, SIGCHLD);
-	sigprocmask (SIG_BLOCK, &tmask, &omask);  
+	gdm_sigchld_block_push ();
 
 	greet = FALSE;
 
@@ -505,7 +502,7 @@ gdm_slave_whack_greeter (void)
 
 	gdm_slave_send_num (GDM_SOP_GREETPID, 0);
 
-	sigprocmask (SIG_SETMASK, &omask, NULL);
+	gdm_sigchld_block_pop ();
 }
 
 /* A hack really, this will wait around until the first mapped window
@@ -591,13 +588,20 @@ focus_first_x_window (const char *class_res_name)
 static void
 run_config (GdmDisplay *display, struct passwd *pwent)
 {
-	gdm_safe_fork (&(display->sesspid));
-	if (display->sesspid < 0) {
+	pid_t pid;
+
+	gdm_sigchld_block_push ();
+	gdm_sigterm_block_push ();
+	pid = d->sesspid = fork ();
+	gdm_sigterm_block_pop ();
+	gdm_sigchld_block_pop ();
+
+	if (pid < 0) {
 		/* can't fork, damnit */
 		display->sesspid = 0;
 		return;
 	}
-	if (display->sesspid == 0) {
+	if (pid == 0) {
 		int i;
 		char **argv;
 		/* child */
@@ -657,8 +661,12 @@ run_config (GdmDisplay *display, struct passwd *pwent)
 
 		_exit (0);
 	} else {
-		waitpid (display->sesspid, 0, 0);
+		gdm_sigchld_block_push ();
+		/* wait for the config proggie to die */
+		if (d->sesspid > 0)
+			waitpid (d->sesspid, 0, 0);
 		display->sesspid = 0;
+		gdm_sigchld_block_pop ();
 	}
 }
 
@@ -1010,6 +1018,7 @@ gdm_slave_greeter (void)
     gchar **argv;
     struct passwd *pwent;
     int i;
+    pid_t pid;
     
     gdm_debug ("gdm_slave_greeter: Running greeter on %s", d->name);
     
@@ -1020,11 +1029,15 @@ gdm_slave_greeter (void)
     if (pipe (pipe1) < 0 || pipe (pipe2) < 0) 
 	gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_greeter: Can't init pipe to gdmgreeter"));
     
+    /* Fork. Parent is gdmslave, child is greeter process. */
+    gdm_sigchld_block_push ();
+    gdm_sigterm_block_push ();
     greet = TRUE;
+    pid = d->greetpid = fork ();
+    gdm_sigterm_block_pop ();
+    gdm_sigchld_block_pop ();
 
-    /* Fork. Parent is gdmslave, child is greeter process. */
-    gdm_safe_fork (&(d->greetpid));
-    switch (d->greetpid) {
+    switch (pid) {
 	
     case 0:
 	sigfillset (&mask);
@@ -1196,7 +1209,7 @@ gdm_slave_greeter (void)
 	close (pipe1[1]);
 	close (pipe2[0]);
 	
-	gdm_debug ("gdm_slave_greeter: Greeter on pid %d", d->greetpid);
+	gdm_debug ("gdm_slave_greeter: Greeter on pid %d", (int)pid);
 
 	gdm_slave_send_num (GDM_SOP_GREETPID, d->greetpid);
 
@@ -1334,6 +1347,7 @@ gdm_slave_chooser (void)
 	char buf[1024];
 	size_t bytes;
 	int i;
+	pid_t pid;
 
 	gdm_debug ("gdm_slave_chooser: Running chooser on %s", d->name);
 
@@ -1345,8 +1359,13 @@ gdm_slave_chooser (void)
 	gdm_slave_exec_script (d, GdmDisplayInit, NULL, NULL);
 
 	/* Fork. Parent is gdmslave, child is greeter process. */
-	gdm_safe_fork (&(d->chooserpid));
-	switch (d->chooserpid) {
+	gdm_sigchld_block_push ();
+	gdm_sigterm_block_push ();
+	pid = d->chooserpid = fork ();
+	gdm_sigterm_block_pop ();
+	gdm_sigchld_block_pop ();
+
+	switch (pid) {
 
 	case 0:
 		sigfillset (&mask);
@@ -1430,8 +1449,12 @@ gdm_slave_chooser (void)
 
 		fcntl(p[0], F_SETFD, fcntl(p[0], F_GETFD, 0) | FD_CLOEXEC);
 
+		gdm_sigchld_block_push ();
 		/* wait for the chooser to die */
-		waitpid (d->chooserpid, 0, 0);
+		if (d->chooserpid > 0)
+			waitpid (d->chooserpid, 0, 0);
+		d->chooserpid  = 0;
+		gdm_sigchld_block_pop ();
 
 		gdm_slave_send_num (GDM_SOP_CHOOSERPID, 0);
 
@@ -1723,224 +1746,21 @@ dequote (const char *in)
 }
 
 static void
-gdm_slave_session_start (void)
+session_child_run (struct passwd *pwent,
+		   const char *session,
+		   const char *save_session,
+		   const char *language,
+		   const char *gnome_session,
+		   gboolean usrcfgok,
+		   gboolean savesess,
+		   gboolean savelang,
+		   gboolean sessoptok,
+		   gboolean savegnomesess)
 {
-    char *cfgdir, *sesspath, *sessexec;
-    struct stat statbuf;
-    struct passwd *pwent;
-    char *save_session = NULL, *session = NULL, *language = NULL, *usrsess, *usrlang;
-    char *gnome_session = NULL;
-    gboolean savesess = FALSE, savelang = FALSE, savegnomesess = FALSE;
-    gboolean usrcfgok = FALSE, sessoptok = FALSE, authok = FALSE;
-    gboolean need_config_sync = FALSE;
-    int i;
-    const char *shell = NULL;
-    pid_t sesspid;
-
-    gdm_debug ("gdm_slave_session_start: Attempting session for user '%s'",
-	       login);
-
-    pwent = getpwnam (login);
-
-    if (pwent == NULL)  {
-	    /* This is sort of an "assert", this should NEVER happen */
-	    if (greet)
-		    gdm_slave_whack_greeter();
-	    gdm_slave_exit (DISPLAY_REMANAGE,
-			    _("gdm_slave_session_start: User passed auth but getpwnam(%s) failed!"), login);
-    }
-
-    if (pwent->pw_dir == NULL ||
-	! g_file_test (pwent->pw_dir, G_FILE_TEST_ISDIR)) {
-	    char *msg = g_strdup_printf (
-		     _("Your home directory is listed as '%s'\n"
-		       "but it does not appear to exist.\n"
-		       "GDM cannot log you in unless you have\n"
-		       "a valid home directory."),
-		     ve_sure_string (pwent->pw_dir));
-	    /* pretend we "logged in" */
-	    if (greet)
-		    gdm_slave_whack_greeter();
-	    /* then tell the user to piss off */
-	    gdm_error_box (d, GNOME_MESSAGE_BOX_ERROR, msg);
-
-	    gdm_error (_("%s: Home directory for %s: '%s' does not exist!"),
-		       "gdm_slave_session_start",
-		       login,
-		       ve_sure_string (pwent->pw_dir));
-	    return;
-    }
-
-    setegid (pwent->pw_gid);
-    seteuid (pwent->pw_uid);
-
-    /* Check if ~user/.gnome exists. Create it otherwise. */
-    cfgdir = g_strconcat (pwent->pw_dir, "/.gnome", NULL);
-    
-    if (stat (cfgdir, &statbuf) == -1) {
-	mkdir (cfgdir, S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
-	chmod (cfgdir, S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
-    }
-    
-    /* Sanity check on ~user/.gnome/gdm */
-    usrcfgok = gdm_file_check ("gdm_slave_session_start", pwent->pw_uid,
-			       cfgdir, "gdm", TRUE, GdmUserMaxFile,
-			       GdmRelaxPerms);
-    /* Sanity check on ~user/.gnome/session-options */
-    sessoptok = gdm_file_check ("gdm_slave_session_start", pwent->pw_uid,
-				cfgdir, "session-options", TRUE, GdmUserMaxFile,
-				/* We cannot be absolutely strict about the
-				 * session permissions, since by default they
-				 * will be writable by group and there's
-				 * nothing we can do about it.  So we relax
-				 * the permission checking in this case */
-				GdmRelaxPerms == 0 ? 1 : GdmRelaxPerms);
-    g_free (cfgdir);
-
-    if (usrcfgok) {
-	gchar *cfgstr;
-
-	cfgstr = g_strconcat ("=", pwent->pw_dir, "/.gnome/gdm=/session/last", NULL);
-	usrsess = gnome_config_get_string (cfgstr);
-	if (usrsess == NULL)
-		usrsess = g_strdup ("");
-	g_free (cfgstr);
-
-	cfgstr = g_strconcat ("=", pwent->pw_dir, "/.gnome/gdm=/session/lang", NULL);
-	usrlang = gnome_config_get_string (cfgstr);
-	if (usrlang == NULL)
-		usrlang = g_strdup ("");
-	g_free (cfgstr);
-    } else {
-	usrsess = g_strdup ("");
-	usrlang = g_strdup ("");
-    }
-
-    seteuid (0);
-    setegid (GdmGroupId);
-
-    if (greet) {
-	    session = gdm_slave_greeter_ctl (GDM_SESS, usrsess);
-	    language = gdm_slave_greeter_ctl (GDM_LANG, usrlang);
-    } else {
-	    session = g_strdup (usrsess);
-	    language = g_strdup (usrlang);
-    }
-
-    g_free (usrsess);
-    g_free (usrlang);
-    
-    if (ve_string_empty (session)) {
-	    g_free (session);
-	    session = find_a_session ();
-	    if (session == NULL) {
-		    /* we're running out of options */
-		    session = g_strdup (GDM_SESSION_FAILSAFE_GNOME);
-	    }
-    }
-
-    if (ve_string_empty (language)) {
-	    const char *lang = g_getenv ("LANG");
-
-	    g_free (language);
-
-	    if (lang != NULL &&
-		lang[0] != '\0') 
-		    language = g_strdup (lang);
-	    else
-		    language = g_strdup (GdmDefaultLocale);
-	    savelang = TRUE;
-
-	    if (ve_string_empty (language)) {
-		    g_free (language);
-		    language = g_strdup ("C");
-	    }
-    }
-
-    /* save this session as the users session */
-    save_session = g_strdup (session);
-
-    if (greet) {
-	    char *ret = gdm_slave_greeter_ctl (GDM_SSESS, "");
-	    if ( ! ve_string_empty (ret))
-		    savesess = TRUE;
-	    g_free (ret);
-
-	    ret = gdm_slave_greeter_ctl (GDM_SLANG, "");
-	    if ( ! ve_string_empty (ret))
-		    savelang = TRUE;
-	    g_free (ret);
-
-	    if (strcmp (session, GDM_SESSION_GNOME_CHOOSER) == 0) {
-		    char *sessions = gdm_get_sessions (pwent);
-		    ret = gdm_slave_greeter_ctl (GDM_GNOMESESS, sessions);
-		    g_free (sessions);
-
-		    if (ret != NULL && ret[0] != '\0') {
-			    gnome_session = ret;
-			    ret = NULL;
-			    g_free (session);
-			    session = g_strdup ("Gnome");
-		    }
-		    g_free (ret);
-
-		    ret = gdm_slave_greeter_ctl (GDM_SGNOMESESS, "");
-		    if ( ! ve_string_empty (ret)) {
-			    savegnomesess = TRUE;
-		    }
-		    g_free (ret);
-	    }
-
-	    gdm_debug (_("gdm_slave_session_start: Authentication completed. Whacking greeter"));
-
-	    gdm_slave_whack_greeter ();
-    }
-
-    /* Ensure some sanity in this world */
-    gdm_ensure_sanity ();
-
-    if (GdmKillInitClients)
-	    gdm_server_whack_clients (d);
-
-    /* setup some env for PreSession script */
-    ve_setenv ("DISPLAY", d->name, TRUE);
-
-    /* If script fails reset X server and restart greeter */
-    if (gdm_slave_exec_script (d, GdmPreSession,
-			       login, pwent) != EXIT_SUCCESS) 
-	gdm_slave_exit (DISPLAY_REMANAGE,
-			_("gdm_slave_session_start: Execution of PreSession script returned > 0. Aborting."));
-
-    /* Setup cookie -- We need this information during cleanup, thus
-     * cookie handling is done before fork()ing */
-
-    setegid (pwent->pw_gid);
-    seteuid (pwent->pw_uid);
-
-    authok = gdm_auth_user_add (d, pwent->pw_uid, pwent->pw_dir);
-
-    seteuid (0);
-    setegid (GdmGroupId);
-    
-    if ( ! authok) {
-	gdm_debug ("gdm_slave_session_start: Auth not OK");
-	gdm_slave_session_stop (0);
-	gdm_slave_session_cleanup ();
-	
-	gdm_server_stop (d);
-	gdm_verify_cleanup (d);
-
-	_exit (DISPLAY_REMANAGE);
-    } 
-
-    /* Start user process */
-    gdm_safe_fork (&(d->sesspid));
-    switch (d->sesspid) {
-	
-    case -1:
-	gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_session_start: Error forking user session"));
-	
-    case 0:
+	int i;
+	char *sesspath, *sessexec;
+	gboolean need_config_sync = FALSE;
+	const char *shell = NULL;
 
 	ve_clearenv ();
 
@@ -1968,9 +1788,7 @@ gdm_slave_session_start (void)
 	 * try unaliasing it */
 	if (strlen (language) < 3 ||
 	    language[2] != '_') {
-		char *newlang = unaliaslang (language);
-		g_free (language);
-		language = newlang;
+		language = unaliaslang (language);
 	}
 
 	/* Set locale */
@@ -2053,13 +1871,9 @@ gdm_slave_session_start (void)
 	 * just change that to "Gnome", since "Gnome Chooser" is a
 	 * fake */
 	if (strcmp (session, GDM_SESSION_GNOME_CHOOSER) == 0) {
-		g_free (session);
-		session = g_strdup ("Gnome");
+		session = "Gnome";
 	}
 	
-	/* Restore sigmask inherited from init */
-	sigprocmask (SIG_SETMASK, &sysmask, NULL);
-	
 	sesspath = NULL;
 	sessexec = NULL;
 
@@ -2070,8 +1884,7 @@ gdm_slave_session_start (void)
 		if (sesspath == NULL) {
 			/* yaikes */
 			gdm_error (_("gdm_slave_session_start: gnome-session not found for a failsafe gnome session, trying xterm"));
-			g_free (session);
-			session = g_strdup (GDM_SESSION_FAILSAFE_XTERM);
+			session = GDM_SESSION_FAILSAFE_XTERM;
 			gdm_error_box
 				(d, GNOME_MESSAGE_BOX_ERROR,
 				 _("Could not find the GNOME installation,\n"
@@ -2170,6 +1983,242 @@ gdm_slave_session_start (void)
 	
 	/* ends as if nothing bad happened */
 	_exit (0);
+}
+
+static void
+gdm_slave_session_start (void)
+{
+    char *cfgdir;
+    struct stat statbuf;
+    struct passwd *pwent;
+    char *save_session = NULL, *session = NULL, *language = NULL, *usrsess, *usrlang;
+    char *gnome_session = NULL;
+    gboolean savesess = FALSE, savelang = FALSE, savegnomesess = FALSE;
+    gboolean usrcfgok = FALSE, sessoptok = FALSE, authok = FALSE;
+    pid_t pid;
+
+    gdm_debug ("gdm_slave_session_start: Attempting session for user '%s'",
+	       login);
+
+    pwent = getpwnam (login);
+
+    if (pwent == NULL)  {
+	    /* This is sort of an "assert", this should NEVER happen */
+	    if (greet)
+		    gdm_slave_whack_greeter();
+	    gdm_slave_exit (DISPLAY_REMANAGE,
+			    _("gdm_slave_session_start: User passed auth but getpwnam(%s) failed!"), login);
+    }
+
+    if (pwent->pw_dir == NULL ||
+	! g_file_test (pwent->pw_dir, G_FILE_TEST_ISDIR)) {
+	    char *msg = g_strdup_printf (
+		     _("Your home directory is listed as '%s'\n"
+		       "but it does not appear to exist.\n"
+		       "GDM cannot log you in unless you have\n"
+		       "a valid home directory."),
+		     ve_sure_string (pwent->pw_dir));
+	    /* pretend we "logged in" */
+	    if (greet)
+		    gdm_slave_whack_greeter();
+	    /* then tell the user to piss off */
+	    gdm_error_box (d, GNOME_MESSAGE_BOX_ERROR, msg);
+
+	    gdm_error (_("%s: Home directory for %s: '%s' does not exist!"),
+		       "gdm_slave_session_start",
+		       login,
+		       ve_sure_string (pwent->pw_dir));
+	    return;
+    }
+
+    setegid (pwent->pw_gid);
+    seteuid (pwent->pw_uid);
+
+    /* Check if ~user/.gnome exists. Create it otherwise. */
+    cfgdir = g_strconcat (pwent->pw_dir, "/.gnome", NULL);
+    
+    if (stat (cfgdir, &statbuf) == -1) {
+	mkdir (cfgdir, S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
+	chmod (cfgdir, S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH);
+    }
+    
+    /* Sanity check on ~user/.gnome/gdm */
+    usrcfgok = gdm_file_check ("gdm_slave_session_start", pwent->pw_uid,
+			       cfgdir, "gdm", TRUE, GdmUserMaxFile,
+			       GdmRelaxPerms);
+    /* Sanity check on ~user/.gnome/session-options */
+    sessoptok = gdm_file_check ("gdm_slave_session_start", pwent->pw_uid,
+				cfgdir, "session-options", TRUE, GdmUserMaxFile,
+				/* We cannot be absolutely strict about the
+				 * session permissions, since by default they
+				 * will be writable by group and there's
+				 * nothing we can do about it.  So we relax
+				 * the permission checking in this case */
+				GdmRelaxPerms == 0 ? 1 : GdmRelaxPerms);
+    g_free (cfgdir);
+
+    if (usrcfgok) {
+	gchar *cfgstr;
+
+	cfgstr = g_strconcat ("=", pwent->pw_dir, "/.gnome/gdm=/session/last", NULL);
+	usrsess = gnome_config_get_string (cfgstr);
+	if (usrsess == NULL)
+		usrsess = g_strdup ("");
+	g_free (cfgstr);
+
+	cfgstr = g_strconcat ("=", pwent->pw_dir, "/.gnome/gdm=/session/lang", NULL);
+	usrlang = gnome_config_get_string (cfgstr);
+	if (usrlang == NULL)
+		usrlang = g_strdup ("");
+	g_free (cfgstr);
+    } else {
+	usrsess = g_strdup ("");
+	usrlang = g_strdup ("");
+    }
+
+    seteuid (0);
+    setegid (GdmGroupId);
+
+    if (greet) {
+	    session = gdm_slave_greeter_ctl (GDM_SESS, usrsess);
+	    language = gdm_slave_greeter_ctl (GDM_LANG, usrlang);
+    } else {
+	    session = g_strdup (usrsess);
+	    language = g_strdup (usrlang);
+    }
+
+    g_free (usrsess);
+    g_free (usrlang);
+    
+    if (ve_string_empty (session)) {
+	    g_free (session);
+	    session = find_a_session ();
+	    if (session == NULL) {
+		    /* we're running out of options */
+		    session = g_strdup (GDM_SESSION_FAILSAFE_GNOME);
+	    }
+    }
+
+    if (ve_string_empty (language)) {
+	    const char *lang = g_getenv ("LANG");
+
+	    g_free (language);
+
+	    if (lang != NULL &&
+		lang[0] != '\0') 
+		    language = g_strdup (lang);
+	    else
+		    language = g_strdup (GdmDefaultLocale);
+	    savelang = TRUE;
+
+	    if (ve_string_empty (language)) {
+		    g_free (language);
+		    language = g_strdup ("C");
+	    }
+    }
+
+    /* save this session as the users session */
+    save_session = g_strdup (session);
+
+    if (greet) {
+	    char *ret = gdm_slave_greeter_ctl (GDM_SSESS, "");
+	    if ( ! ve_string_empty (ret))
+		    savesess = TRUE;
+	    g_free (ret);
+
+	    ret = gdm_slave_greeter_ctl (GDM_SLANG, "");
+	    if ( ! ve_string_empty (ret))
+		    savelang = TRUE;
+	    g_free (ret);
+
+	    if (strcmp (session, GDM_SESSION_GNOME_CHOOSER) == 0) {
+		    char *sessions = gdm_get_sessions (pwent);
+		    ret = gdm_slave_greeter_ctl (GDM_GNOMESESS, sessions);
+		    g_free (sessions);
+
+		    if (ret != NULL && ret[0] != '\0') {
+			    gnome_session = ret;
+			    ret = NULL;
+			    g_free (session);
+			    session = g_strdup ("Gnome");
+		    }
+		    g_free (ret);
+
+		    ret = gdm_slave_greeter_ctl (GDM_SGNOMESESS, "");
+		    if ( ! ve_string_empty (ret)) {
+			    savegnomesess = TRUE;
+		    }
+		    g_free (ret);
+	    }
+
+	    gdm_debug (_("gdm_slave_session_start: Authentication completed. Whacking greeter"));
+
+	    gdm_slave_whack_greeter ();
+    }
+
+    /* Ensure some sanity in this world */
+    gdm_ensure_sanity ();
+
+    if (GdmKillInitClients)
+	    gdm_server_whack_clients (d);
+
+    /* setup some env for PreSession script */
+    ve_setenv ("DISPLAY", d->name, TRUE);
+
+    /* If script fails reset X server and restart greeter */
+    if (gdm_slave_exec_script (d, GdmPreSession,
+			       login, pwent) != EXIT_SUCCESS) 
+	gdm_slave_exit (DISPLAY_REMANAGE,
+			_("gdm_slave_session_start: Execution of PreSession script returned > 0. Aborting."));
+
+    /* Setup cookie -- We need this information during cleanup, thus
+     * cookie handling is done before fork()ing */
+
+    setegid (pwent->pw_gid);
+    seteuid (pwent->pw_uid);
+
+    authok = gdm_auth_user_add (d, pwent->pw_uid, pwent->pw_dir);
+
+    seteuid (0);
+    setegid (GdmGroupId);
+    
+    if ( ! authok) {
+	gdm_debug ("gdm_slave_session_start: Auth not OK");
+	gdm_slave_session_stop (0);
+	gdm_slave_session_cleanup ();
+	
+	gdm_server_stop (d);
+	gdm_verify_cleanup (d);
+
+	_exit (DISPLAY_REMANAGE);
+    } 
+
+    /* Start user process */
+    gdm_sigchld_block_push ();
+    gdm_sigterm_block_push ();
+    pid = d->sesspid = fork ();
+    gdm_sigterm_block_pop ();
+    gdm_sigchld_block_pop ();
+
+    switch (pid) {
+	
+    case -1:
+	gdm_slave_exit (DISPLAY_ABORT, _("gdm_slave_session_start: Error forking user session"));
+	
+    case 0:
+
+	/* Never returns */
+	session_child_run (pwent,
+			   session,
+			   save_session,
+			   language,
+			   gnome_session,
+			   usrcfgok,
+			   savesess,
+			   savelang,
+			   sessoptok,
+			   savegnomesess);
+	g_assert_not_reached ();
 	
     default:
 	break;
@@ -2180,18 +2229,24 @@ gdm_slave_session_start (void)
     g_free (language);
     g_free (gnome_session);
 
-    sesspid = d->sesspid;
-    gdm_slave_send_num (GDM_SOP_SESSPID, sesspid);
+    /* we block sigchld now that we're here */
+    gdm_sigchld_block_push ();
+
+    if (d->sesspid > 0) {
+	    gdm_slave_send_num (GDM_SOP_SESSPID, pid);
+
+	    d->sesspid = 0;
+
+	    gdm_sigchld_block_pop ();
 
-    /* Wait for the user's session to exit, but by this time the
-     * session might have ended, so check for 0 */
-    if (d->sesspid > 0)
-	    waitpid (d->sesspid, NULL, 0);
-    d->sesspid = 0;
+	    waitpid (pid, NULL, 0);
+    } else {
+	    gdm_sigchld_block_pop ();
+    }
 
     gdm_debug ("gdm_slave_session_start: Session ended OK");
 
-    gdm_slave_session_stop (sesspid);
+    gdm_slave_session_stop (pid);
     gdm_slave_session_cleanup ();
 }
 
@@ -2388,6 +2443,7 @@ gdm_slave_child_handler (int sig)
 	} else if (pid == extra_process) {
 		/* an extra process died, yay! */
 		extra_process = -1;
+	    	extra_status = status;
 	}
     }
 }
@@ -2587,8 +2643,8 @@ gdm_slave_exec_script (GdmDisplay *d, const gchar *dir, const char *login,
     else
 	return EXIT_SUCCESS;
 
-    gdm_safe_fork (&extra_process);
-    pid = extra_process;
+    pid = gdm_fork_extra ();
+
     switch (pid) {
 	    
     case 0:
@@ -2634,9 +2690,7 @@ gdm_slave_exec_script (GdmDisplay *d, const gchar *dir, const char *login,
 	return EXIT_SUCCESS;
 	
     default:
-	waitpid (pid, &status, 0);	/* Wait for script to finish */
-
-	extra_process = -1;
+	gdm_wait_for_extra (&status);
 
 	if (WIFEXITED (status))
 	    return WEXITSTATUS (status);
@@ -2710,7 +2764,6 @@ gdm_parse_enriched_login (const gchar *s, GdmDisplay *display)
     gint pipe1[2], in_buffer_len;  
     gchar **argv;
     pid_t pid;
-    gint status;
 
     if (s == NULL)
 	return(NULL);
@@ -2756,8 +2809,8 @@ gdm_parse_enriched_login (const gchar *s, GdmDisplay *display)
       if (pipe (pipe1) < 0) {
         gdm_error (_("gdm_parse_enriched_login: Failed creating pipe"));
       } else {
-	gdm_safe_fork (&extra_process);
-	pid = extra_process;
+	pid = gdm_fork_extra ();
+
         switch (pid) {
 	    
         case 0:
@@ -2800,8 +2853,8 @@ gdm_parse_enriched_login (const gchar *s, GdmDisplay *display)
               g_string_truncate(str, str->len - 1);
 
             close(pipe1[0]);
-	    waitpid (pid, &status, 0);	/* Wait for script to finish */
-	    extra_process = -1;
+
+	    gdm_wait_for_extra (NULL);
         }
       }
     }