make signal flags volatile, simplify handling

chansession.h

@@ -94,6 +94,7 @@ void cli_send_netcat_request(void);
 #endif
 
 void svr_chansessinitialise(void);
+void svr_chansess_checksignal(void);
 extern const struct ChanType svrchansess;
 
 struct SigMap {

cli-main.c

@@ -108,7 +108,7 @@ static void cli_dropbear_exit(int exitcode, const char* format, va_list param) {
 	vsnprintf(exitmsg, sizeof(exitmsg), format, param);
 
 	/* Add the prefix depending on session/auth state */
-	if (!sessinitdone) {
+	if (!ses.init_done) {
 		snprintf(fullmsg, sizeof(fullmsg), "Exited: %s", exitmsg);
 	} else {
 		snprintf(fullmsg, sizeof(fullmsg), 

cli-session.c

@@ -118,7 +118,7 @@ void cli_session(int sock_in, int sock_out, struct dropbear_progress_connection
 	cli_session_init(proxy_cmd_pid);
 
 	/* Ready to go */
-	sessinitdone = 1;
+	ses.init_done = 1;
 
 	/* Exchange identification */
 	send_session_identification();
@@ -338,7 +338,7 @@ void kill_proxy_command(void) {
 
 static void cli_session_cleanup(void) {
 
-	if (!sessinitdone) {
+	if (!ses.init_done) {
 		return;
 	}
 

common-session.c

@@ -43,13 +43,6 @@ static void read_session_identification(void);
 
 struct sshsession ses; /* GLOBAL */
 
-/* need to know if the session struct has been initialised, this way isn't the
- * cleanest, but works OK */
-int sessinitdone = 0; /* GLOBAL */
-
-/* this is set when we get SIGINT or SIGTERM, the handler is in main.c */
-int exitflag = 0; /* GLOBAL */
-
 /* called only at the start of a session, set up initial state */
 void common_session_init(int sock_in, int sock_out) {
 	time_t now;
@@ -162,7 +155,6 @@ void session_loop(void(*loophandler)()) {
 		/* We get woken up when signal handlers write to this pipe.
 		   SIGCHLD in svr-chansession is the only one currently. */
 		FD_SET(ses.signal_pipe[0], &readfd);
-		ses.channel_signal_pending = 0;
 
 		/* set up for channels which can be read/written */
 		setchannelfds(&readfd, &writefd, writequeue_has_space);
@@ -190,7 +182,7 @@ void session_loop(void(*loophandler)()) {
 
 		val = select(ses.maxfd+1, &readfd, &writefd, NULL, &timeout);
 
-		if (exitflag) {
+		if (ses.exitflag) {
 			dropbear_exit("Terminated by signal");
 		}
 		
@@ -210,6 +202,7 @@ void session_loop(void(*loophandler)()) {
 		/* We'll just empty out the pipe if required. We don't do
 		any thing with the data, since the pipe's purpose is purely to
 		wake up the select() above. */
+		ses.channel_signal_pending = 0;
 		if (FD_ISSET(ses.signal_pipe[0], &readfd)) {
 			char x;
 			TRACE(("signal pipe set"))
@@ -244,6 +237,10 @@ void session_loop(void(*loophandler)()) {
 
 		handle_connect_fds(&writefd);
 
+		/* loop handler prior to channelio, in case the server loophandler closes
+		channels on process exit */
+		loophandler();
+
 		/* process pipes etc for the channels, ses.dataallowed == 0
 		 * during rekeying ) */
 		channelio(&readfd, &writefd);
@@ -255,11 +252,6 @@ void session_loop(void(*loophandler)()) {
 			}
 		}
 
-
-		if (loophandler) {
-			loophandler();
-		}
-
 	} /* for(;;) */
 	
 	/* Not reached */
@@ -280,8 +272,8 @@ void session_cleanup() {
 	TRACE(("enter session_cleanup"))
 	
 	/* we can't cleanup if we don't know the session state */
-	if (!sessinitdone) {
-		TRACE(("leave session_cleanup: !sessinitdone"))
+	if (!ses.init_done) {
+		TRACE(("leave session_cleanup: !ses.init_done"))
 		return;
 	}
 

session.h

@@ -40,9 +40,6 @@
 #include "dbutil.h"
 #include "netio.h"
 
-extern int sessinitdone; /* Is set to 0 somewhere */
-extern int exitflag;
-
 void common_session_init(int sock_in, int sock_out);
 void session_loop(void(*loophandler)()) ATTRIB_NORETURN;
 void session_cleanup(void);
@@ -157,6 +154,7 @@ struct sshsession {
 	
 	int signal_pipe[2]; /* stores endpoints of a self-pipe used for
 						   race-free signal handling */
+	int channel_signal_pending; /* Flag set when the signal pipe is triggered */
 
 	m_list conn_pending;
 						
@@ -203,7 +201,6 @@ struct sshsession {
 	unsigned int chansize; /* the number of Channel*s allocated for channels */
 	unsigned int chancount; /* the number of Channel*s in use */
 	const struct ChanType **chantypes; /* The valid channel types */
-	int channel_signal_pending; /* Flag set by sigchld handler */
 
 	/* TCP priority level for the main "port 22" tcp socket */
 	enum dropbear_prio socket_prio;
@@ -216,6 +213,10 @@ struct sshsession {
 	 * really belong here, but nowhere else fits nicely */
 	int allowprivport;
 
+	/* this is set when we get SIGINT or SIGTERM, the handler is in main.c */
+	volatile int exitflag;
+	/* set once the ses structure (and cli_ses/svr_ses) have been populated to their initial state */
+	int init_done;
 };
 
 struct serversession {
@@ -283,7 +284,7 @@ struct clientsession {
 	/* for escape char handling */
 	int last_char;
 
-	int winchange; /* Set to 1 when a windowchange signal happens */
+	volatile int winchange; /* Set to 1 when a windowchange signal happens */
 
 	int lastauthtype; /* either AUTH_TYPE_PUBKEY or AUTH_TYPE_PASSWORD,
 						 for the last type of auth we tried */

svr-chansession.c

@@ -80,6 +80,55 @@ static int sesscheckclose(const struct Channel *channel) {
 	return chansess->exit.exitpid != -1;
 }
 
+void svr_chansess_checksignal(void) {
+	int status;
+	pid_t pid;
+
+	if (!ses.channel_signal_pending) {
+		return;
+	}
+
+	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
+		unsigned int i;
+		struct exitinfo *ex = NULL;
+		TRACE(("sigchld handler: pid %d", pid))
+
+		ex = NULL;
+		/* find the corresponding chansess */
+		for (i = 0; i < svr_ses.childpidsize; i++) {
+			if (svr_ses.childpids[i].pid == pid) {
+				TRACE(("found match session"));
+				ex = &svr_ses.childpids[i].chansess->exit;
+				break;
+			}
+		}
+
+		/* If the pid wasn't matched, then we might have hit the race mentioned
+		 * above. So we just store the info for the parent to deal with */
+		if (ex == NULL) {
+			TRACE(("using lastexit"));
+			ex = &svr_ses.lastexit;
+		}
+
+		ex->exitpid = pid;
+		if (WIFEXITED(status)) {
+			ex->exitstatus = WEXITSTATUS(status);
+		}
+		if (WIFSIGNALED(status)) {
+			ex->exitsignal = WTERMSIG(status);
+#if !defined(AIX) && defined(WCOREDUMP)
+			ex->exitcore = WCOREDUMP(status);
+#else
+			ex->exitcore = 0;
+#endif
+		} else {
+			/* we use this to determine how pid exited */
+			ex->exitsignal = -1;
+		}
+		
+	}
+}
+
 /* Handler for childs exiting, store the state for return to the client */
 
 /* There's a particular race we have to watch out for: if the forked child
@@ -89,63 +138,20 @@ static int sesscheckclose(const struct Channel *channel) {
  * the parent when it runs. This work correctly at least in the case of a
  * single shell spawned (ie the usual case) */
 static void sesssigchild_handler(int UNUSED(dummy)) {
-
-	int status;
-	pid_t pid;
 	unsigned int i;
 	struct sigaction sa_chld;
-	struct exitinfo *exit = NULL;
 
 	const int saved_errno = errno;
 
-	/* Make channel handling code look for closed channels */
-	ses.channel_signal_pending = 1;
-
 	TRACE(("enter sigchld handler"))
-	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
-		TRACE(("sigchld handler: pid %d", pid))
 
-		exit = NULL;
-		/* find the corresponding chansess */
-		for (i = 0; i < svr_ses.childpidsize; i++) {
-			if (svr_ses.childpids[i].pid == pid) {
-				TRACE(("found match session"));
-				exit = &svr_ses.childpids[i].chansess->exit;
-				break;
-			}
-		}
-
-		/* If the pid wasn't matched, then we might have hit the race mentioned
-		 * above. So we just store the info for the parent to deal with */
-		if (exit == NULL) {
-			TRACE(("using lastexit"));
-			exit = &svr_ses.lastexit;
-		}
-
-		exit->exitpid = pid;
-		if (WIFEXITED(status)) {
-			exit->exitstatus = WEXITSTATUS(status);
-		}
-		if (WIFSIGNALED(status)) {
-			exit->exitsignal = WTERMSIG(status);
-#if !defined(AIX) && defined(WCOREDUMP)
-			exit->exitcore = WCOREDUMP(status);
-#else
-			exit->exitcore = 0;
-#endif
-		} else {
-			/* we use this to determine how pid exited */
-			exit->exitsignal = -1;
-		}
-		
-		/* Make sure that the main select() loop wakes up */
-		while (1) {
-			/* isserver is just a random byte to write. We can't do anything
-			about an error so should just ignore it */
-			if (write(ses.signal_pipe[1], &ses.isserver, 1) == 1
-					|| errno != EINTR) {
-				break;
-			}
+	/* Make sure that the main select() loop wakes up */
+	while (1) {
+		/* isserver is just a random byte to write. We can't do anything
+		about an error so should just ignore it */
+		if (write(ses.signal_pipe[1], &ses.isserver, 1) == 1
+				|| errno != EINTR) {
+			break;
 		}
 	}
 

svr-main.c

@@ -188,7 +188,7 @@ static void main_noinetd() {
 
 		val = select(maxsock+1, &fds, NULL, NULL, NULL);
 
-		if (exitflag) {
+		if (ses.exitflag) {
 			unlink(svr_opts.pidfile);
 			dropbear_exit("Terminated by signal");
 		}
@@ -359,7 +359,7 @@ static void sigsegv_handler(int UNUSED(unused)) {
 /* catch ctrl-c or sigterm */
 static void sigintterm_handler(int UNUSED(unused)) {
 
-	exitflag = 1;
+	ses.exitflag = 1;
 }
 
 /* Things used by inetd and non-inetd modes */

svr-session.c

@@ -124,7 +124,7 @@ void svr_session(int sock, int childpipe) {
 	ses.isserver = 1;
 
 	/* We're ready to go now */
-	sessinitdone = 1;
+	ses.init_done = 1;
 
 	/* exchange identification, version etc */
 	send_session_identification();
@@ -136,7 +136,7 @@ void svr_session(int sock, int childpipe) {
 
 	/* Run the main for loop. NULL is for the dispatcher - only the client
 	 * code makes use of it */
-	session_loop(NULL);
+	session_loop(svr_chansess_checksignal);
 
 	/* Not reached */
 
@@ -152,7 +152,7 @@ void svr_dropbear_exit(int exitcode, const char* format, va_list param) {
 	vsnprintf(exitmsg, sizeof(exitmsg), format, param);
 
 	/* Add the prefix depending on session/auth state */
-	if (!sessinitdone) {
+	if (!ses.init_done) {
 		/* before session init */
 		snprintf(fullmsg, sizeof(fullmsg), "Early exit: %s", exitmsg);
 	} else if (ses.authstate.authdone) {