Split ChanType closehandler() and cleanup() so that dbclient doesn't

lose exit status messages

Author: mkj

channel.h

@@ -69,10 +69,6 @@ struct Channel {
 	int sent_close, recv_close;
 	int recv_eof, sent_eof;
 
-	/* Set after running the ChanType-specific close hander
-	 * to ensure we don't run it twice (nor type->checkclose()). */
-	int close_handler_done;
-
 	struct dropbear_progress_connection *conn_pending;
 	int initconn; /* used for TCP forwarding, whether the channel has been
 					 fully initialised */
@@ -95,10 +91,17 @@ struct ChanType {
 
 	int sepfds; /* Whether this channel has separate pipes for in/out or not */
 	const char *name;
+	/* Sets up the channel */
 	int (*inithandler)(struct Channel*);
+	/* Called to check whether a channel should close, separately from the FD being closed.
+	Used for noticing process exiting */
 	int (*check_close)(const struct Channel*);
+	/* Handler for ssh_msg_channel_request */
 	void (*reqhandler)(struct Channel*);
+	/* Called prior to sending ssh_msg_channel_close, used for sending exit status */
 	void (*closehandler)(const struct Channel*);
+	/* Frees resources, called just prior to channel being removed */
+	void (*cleanup)(const struct Channel*);
 };
 
 /* Callback for connect_remote */

cli-agentfwd.c

@@ -52,6 +52,7 @@ const struct ChanType cli_chan_agent = {
 	new_agent_chan,
 	NULL,
 	NULL,
+	NULL,
 	NULL
 };
 

cli-chansession.c

@@ -35,7 +35,7 @@
 #include "chansession.h"
 #include "agentfwd.h"
 
-static void cli_closechansess(const struct Channel *channel);
+static void cli_cleanupchansess(const struct Channel *channel);
 static int cli_initchansess(struct Channel *channel);
 static void cli_chansessreq(struct Channel *channel);
 static void send_chansess_pty_req(const struct Channel *channel);
@@ -51,7 +51,8 @@ const struct ChanType clichansess = {
 	cli_initchansess, /* inithandler */
 	NULL, /* checkclosehandler */
 	cli_chansessreq, /* reqhandler */
-	cli_closechansess, /* closehandler */
+	NULL, /* closehandler */
+	cli_cleanupchansess, /* cleanup */
 };
 
 static void cli_chansessreq(struct Channel *channel) {
@@ -83,7 +84,7 @@ static void cli_chansessreq(struct Channel *channel) {
 
 
 /* If the main session goes, we close it up */
-static void cli_closechansess(const struct Channel *UNUSED(channel)) {
+static void cli_cleanupchansess(const struct Channel *UNUSED(channel)) {
 	cli_tty_cleanup(); /* Restore tty modes etc */
 
 	/* This channel hasn't gone yet, so we have > 1 */
@@ -387,7 +388,8 @@ static const struct ChanType cli_chan_netcat = {
 	cli_init_netcat, /* inithandler */
 	NULL,
 	NULL,
-	cli_closechansess
+	NULL,
+	cli_cleanupchansess
 };
 
 void cli_send_netcat_request() {

cli-session.c

@@ -356,6 +356,7 @@ static void cli_session_cleanup(void) {
 }
 
 static void cli_finished() {
+	TRACE(("cli_finised()"))
 
 	session_cleanup();
 	fprintf(stderr, "Connection to %s@%s:%s closed.\n", cli_opts.username,

cli-tcpfwd.c

@@ -40,6 +40,7 @@ const struct ChanType cli_chan_tcpremote = {
 	newtcpforwarded,
 	NULL,
 	NULL,
+	NULL,
 	NULL
 };
 #endif
@@ -55,6 +56,7 @@ static const struct ChanType cli_chan_tcplocal = {
 	tcp_prio_inithandler,
 	NULL,
 	NULL,
+	NULL,
 	NULL
 };
 #endif

common-channel.c

@@ -144,7 +144,6 @@ static struct Channel* newchannel(unsigned int remotechan,
 	newchan->index = i;
 	newchan->sent_close = newchan->recv_close = 0;
 	newchan->sent_eof = newchan->recv_eof = 0;
-	newchan->close_handler_done = 0;
 
 	newchan->remotechan = remotechan;
 	newchan->transwindow = transwindow;
@@ -286,7 +285,7 @@ static void check_close(struct Channel *channel) {
 				channel->extrabuf ? cbuf_getused(channel->extrabuf) : 0))
 
 	if (!channel->flushing 
-		&& !channel->close_handler_done
+		&& !channel->sent_close
 		&& channel->type->check_close
 		&& channel->type->check_close(channel))
 	{
@@ -298,7 +297,7 @@ static void check_close(struct Channel *channel) {
 	   channel, to ensure that the shell has exited (and the exit status 
 	   retrieved) before we close things up. */
 	if (!channel->type->check_close	
-		|| channel->close_handler_done
+		|| channel->sent_close
 		|| channel->type->check_close(channel)) {
 		close_allowed = 1;
 	}
@@ -385,10 +384,8 @@ void channel_connect_done(int result, int sock, void* user_data, const char* UNU
 static void send_msg_channel_close(struct Channel *channel) {
 
 	TRACE(("enter send_msg_channel_close %p", (void*)channel))
-	if (channel->type->closehandler 
-			&& !channel->close_handler_done) {
+	if (channel->type->closehandler) {
 		channel->type->closehandler(channel);
-		channel->close_handler_done = 1;
 	}
 
 	CHECKCLEARTOWRITE();
@@ -661,10 +658,8 @@ static void remove_channel(struct Channel * channel) {
 		m_close(channel->errfd);
 	}
 
-	if (!channel->close_handler_done
-		&& channel->type->closehandler) {
-		channel->type->closehandler(channel);
-		channel->close_handler_done = 1;
+	if (channel->type->cleanup) {
+		channel->type->cleanup(channel);
 	}
 
 	if (channel->conn_pending) {
@@ -690,13 +685,7 @@ void recv_msg_channel_request() {
 
 	TRACE(("enter recv_msg_channel_request %p", (void*)channel))
 
-	if (channel->sent_close) {
-		TRACE(("leave recv_msg_channel_request: already closed channel"))
-		return;
-	}
-
-	if (channel->type->reqhandler 
-			&& !channel->close_handler_done) {
+	if (channel->type->reqhandler) {
 		channel->type->reqhandler(channel);
 	} else {
 		int wantreply;
@@ -1011,6 +1000,11 @@ void recv_msg_channel_open() {
 void send_msg_channel_failure(const struct Channel *channel) {
 
 	TRACE(("enter send_msg_channel_failure"))
+
+	if (channel->sent_close) {
+		TRACE(("Skipping sending msg_channel_failure for closed channel"))
+		return;
+	}
 	CHECKCLEARTOWRITE();
 
 	buf_putbyte(ses.writepayload, SSH_MSG_CHANNEL_FAILURE);
@@ -1024,6 +1018,10 @@ void send_msg_channel_failure(const struct Channel *channel) {
 void send_msg_channel_success(const struct Channel *channel) {
 
 	TRACE(("enter send_msg_channel_success"))
+	if (channel->sent_close) {
+		TRACE(("Skipping sending msg_channel_success for closed channel"))
+		return;
+	}
 	CHECKCLEARTOWRITE();
 
 	buf_putbyte(ses.writepayload, SSH_MSG_CHANNEL_SUCCESS);

svr-agentfwd.c

@@ -187,6 +187,7 @@ static const struct ChanType chan_svr_agent = {
 	NULL,
 	NULL,
 	NULL,
+	NULL,
 	NULL
 };
 

svr-chansession.c

@@ -51,6 +51,7 @@ static void execchild(const void *user_data_chansess);
 static void addchildpid(struct ChanSess *chansess, pid_t pid);
 static void sesssigchild_handler(int val);
 static void closechansess(const struct Channel *channel);
+static void cleanupchansess(const struct Channel *channel);
 static int newchansess(struct Channel *channel);
 static void chansessionrequest(struct Channel *channel);
 static int sesscheckclose(const struct Channel *channel);
@@ -69,6 +70,7 @@ const struct ChanType svrchansess = {
 	sesscheckclose, /* checkclosehandler */
 	chansessionrequest, /* reqhandler */
 	closechansess, /* closehandler */
+	cleanupchansess /* cleanup */
 };
 
 /* required to clear environment */
@@ -91,7 +93,7 @@ void svr_chansess_checksignal(void) {
 	while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
 		unsigned int i;
 		struct exitinfo *ex = NULL;
-		TRACE(("sigchld handler: pid %d", pid))
+		TRACE(("svr_chansess_checksignal : pid %d", pid))
 
 		ex = NULL;
 		/* find the corresponding chansess */
@@ -285,8 +287,25 @@ chansess_login_alloc(const struct ChanSess *chansess) {
 	return li;
 }
 
-/* clean a session channel */
+/* send exit status message before the channel is closed */
 static void closechansess(const struct Channel *channel) {
+	struct ChanSess *chansess;
+
+	TRACE(("enter closechansess"))
+
+	chansess = (struct ChanSess*)channel->typedata;
+
+	if (chansess == NULL) {
+		TRACE(("leave closechansess: chansess == NULL"))
+		return;
+	}
+
+	send_exitsignalstatus(channel);
+	TRACE(("leave closechansess"))
+}
+
+/* clean a session channel */
+static void cleanupchansess(const struct Channel *channel) {
 
 	struct ChanSess *chansess;
 	unsigned int i;
@@ -301,8 +320,6 @@ static void closechansess(const struct Channel *channel) {
 		return;
 	}
 
-	send_exitsignalstatus(channel);
-
 	m_free(chansess->cmd);
 	m_free(chansess->term);
 

svr-tcpfwd.c

@@ -238,7 +238,8 @@ const struct ChanType svr_chan_tcpdirect = {
 	newtcpdirect, /* init */
 	NULL, /* checkclose */
 	NULL, /* reqhandler */
-	NULL /* closehandler */
+	NULL, /* closehandler */
+	NULL /* cleanup */
 };
 
 /* Called upon creating a new direct tcp channel (ie we connect out to an

svr-x11fwd.c

@@ -216,7 +216,8 @@ static const struct ChanType chan_x11 = {
 	x11_inithandler, /* inithandler */
 	NULL, /* checkclose */
 	NULL, /* reqhandler */
-	NULL /* closehandler */
+	NULL, /* closehandler */
+	NULL /* cleanup */
 };