- Fix bug in child-exit handling where the wrong pid was being matched.

- Also wait for errfd to close before closing the channel

--HG--
branch : channel-fix
extra : convert_revision : ed68ea8963f7d1fa600f81479e7e211efc5a31bc

common-channel.c

@@ -313,6 +313,7 @@ static void check_close(struct Channel *channel) {
 	/* And if we can't receive any more data from them either, close up */
 	if (!channel->sent_close
 			&& channel->readfd == FD_CLOSED
+			&& (ERRFD_IS_WRITE(channel) || channel->errfd == FD_CLOSED)
 			&& !write_pending(channel)) {
 		TRACE(("sending close, readfd is closed"))
 		send_msg_channel_close(channel);
@@ -624,8 +625,8 @@ static void send_msg_channel_data(struct Channel *channel, int isextended) {
 			close_chan_fd(channel, fd, SHUT_RD);
 		}
 		ses.writepayload->len = ses.writepayload->pos = 0;
-		TRACE(("leave send_msg_channel_data: len %d read err or EOF for fd %d", 
+		TRACE(("leave send_msg_channel_data: len %d read err %d or EOF for fd %d", 
-					len, channel->index));
+					len, errno, fd))
 		return;
 	}
 	buf_incrwritepos(ses.writepayload, len);

svr-chansession.c

@@ -67,6 +67,7 @@ extern char** environ;
 
 static int sesscheckclose(struct Channel *channel) {
 	struct ChanSess *chansess = (struct ChanSess*)channel->typedata;
+	TRACE(("sesscheckclose, pid is %d", chansess->exit.exitpid))
 	return chansess->exit.exitpid != -1;
 }
 
@@ -88,12 +89,13 @@ static void sesssigchild_handler(int UNUSED(dummy)) {
 
 	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;
 			}
@@ -102,6 +104,7 @@ static void sesssigchild_handler(int UNUSED(dummy)) {
 		/* 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;
 		}
 
@@ -125,7 +128,8 @@ static void sesssigchild_handler(int UNUSED(dummy)) {
 		while (1) {
 			/* EAGAIN means the pipe's full, so don't need to write anything */
 			/* isserver is just a random byte to write */
-			if (write(ses.signal_pipe[1], &ses.isserver, 1) == 1 || errno == EAGAIN) {
+			if (write(ses.signal_pipe[1], &ses.isserver, 1) == 1 
+					|| errno == EAGAIN) {
 				break;
 			}
 			if (errno == EINTR) {
@@ -142,7 +146,6 @@ static void sesssigchild_handler(int UNUSED(dummy)) {
 }
 
 /* send the exit status or the signal causing termination for a session */
-/* XXX server */
 static void send_exitsignalstatus(struct Channel *channel) {
 
 	struct ChanSess *chansess = (struct ChanSess*)channel->typedata;
@@ -181,10 +184,11 @@ static void send_msg_chansess_exitsignal(struct Channel * channel,
 
 	int i;
 	char* signame = NULL;
-
 	dropbear_assert(chansess->exit.exitpid != -1);
 	dropbear_assert(chansess->exit.exitsignal > 0);
 
+	TRACE(("send_msg_chansess_exitsignal %d", chansess->exit.exitsignal))
+
 	CHECKCLEARTOWRITE();
 
 	/* we check that we can match a signal name, otherwise
@@ -294,7 +298,7 @@ static void closechansess(struct Channel *channel) {
 		if (svr_ses.childpids[i].chansess == chansess) {
 			dropbear_assert(svr_ses.childpids[i].pid > 0);
 			TRACE(("closing pid %d", svr_ses.childpids[i].pid))
-			TRACE(("exitpid = %d", chansess->exit.exitpid))
+			TRACE(("exitpid is %d", chansess->exit.exitpid))
 			svr_ses.childpids[i].pid = -1;
 			svr_ses.childpids[i].chansess = NULL;
 		}
@@ -682,22 +686,24 @@ static int noptycommand(struct Channel *channel, struct ChanSess *chansess) {
 		/* parent */
 		TRACE(("continue noptycommand: parent"))
 		chansess->pid = pid;
+		TRACE(("child pid is %d", pid))
 
 		addchildpid(chansess, pid);
 
 		if (svr_ses.lastexit.exitpid != -1) {
+			TRACE(("parent side: lastexitpid is %d", svr_ses.lastexit.exitpid))
 			/* The child probably exited and the signal handler triggered
 			 * possibly before we got around to adding the childpid. So we fill
-			 * out it's data manually */
+			 * out its data manually */
 			for (i = 0; i < svr_ses.childpidsize; i++) {
-				if (svr_ses.childpids[i].pid == pid) {
+				if (svr_ses.childpids[i].pid == svr_ses.lastexit.exitpid) {
+					TRACE(("found match for lastexitpid"))
 					svr_ses.childpids[i].chansess->exit = svr_ses.lastexit;
 					svr_ses.lastexit.exitpid = -1;
 				}
 			}
 		}
 
-
 		close(infds[FDIN]);
 		close(outfds[FDOUT]);
 		close(errfds[FDOUT]);