OpenSSH solaris: bad return code after exec of remote command

Markus Friedl markus at openbsd.org
Thu Oct 11 06:22:50 EST 2001


I wrote:

> it's easy to reproduce your problem:
> 
> ssh -n localhost 'echo x; exec > /dev/null 2>&1; sleep 3; exit 5;'

could you please try this diff against the current CVS

hope everything else stil works...

-m

Index: channels.c
===================================================================
RCS file: /cvs/openssh_cvs/channels.c,v
retrieving revision 1.110
diff -u -r1.110 channels.c
--- channels.c	2001/10/10 05:14:38	1.110
+++ channels.c	2001/10/10 20:26:54
@@ -331,10 +331,6 @@
 	debug3("channel_free: status: %s", s);
 	xfree(s);
 
-	if (c->detach_user != NULL) {
-		debug("channel_free: channel %d: detaching channel user", c->self);
-		c->detach_user(c->self, NULL);
-	}
 	if (c->sock != -1)
 		shutdown(c->sock, SHUT_RDWR);
 	channel_close_fds(c);
@@ -1520,6 +1516,30 @@
 		channel_handler_init_15();
 }
 
+/* gc dead channels */
+static void
+channel_garbage_collect(Channel *c)
+{
+	if (c == NULL)
+		return;
+	debug("channel %d: gc called", c->self);
+	if (c->detach_user != NULL) {
+		if (!chan_is_dead(c, 0))
+			return;
+		debug("channel %d: gc: notify user", c->self);
+		c->detach_user(c->self, NULL);
+		/* if we still have a callback */
+		if (c->detach_user != NULL)
+			return;
+		debug("channel %d: gc: user removed callback", c->self);
+	}
+	debug("channel %d: gc: after notify user", c->self);
+	if (!chan_is_dead(c, 1))
+		return;
+	debug("channel %d: garbage collecting", c->self);
+	channel_free(c);
+}
+
 static void
 channel_handler(chan_fn *ftab[], fd_set * readset, fd_set * writeset)
 {
@@ -1537,24 +1557,7 @@
 			continue;
 		if (ftab[c->type] != NULL)
 			(*ftab[c->type])(c, readset, writeset);
-		if (chan_is_dead(c)) {
-			/*
-			 * we have to remove the fd's from the select mask
-			 * before the channels are free'd and the fd's are
-			 * closed
-			 */
-			if (c->wfd != -1)
-				FD_CLR(c->wfd, writeset);
-			if (c->rfd != -1)
-				FD_CLR(c->rfd, readset);
-			if (c->efd != -1) {
-				if (c->extended_usage == CHAN_EXTENDED_READ)
-					FD_CLR(c->efd, readset);
-				if (c->extended_usage == CHAN_EXTENDED_WRITE)
-					FD_CLR(c->efd, writeset);
-			}
-			channel_free(c);
-		}
+		channel_garbage_collect(c);
 	}
 }
 
@@ -1625,7 +1628,7 @@
 		if (compat20 &&
 		    (c->flags & (CHAN_CLOSE_SENT|CHAN_CLOSE_RCVD))) {
 			/* XXX is this true? */
-			debug2("channel %d: no data after CLOSE", c->self);
+			debug3("channel %d: will not send data after CLOSE", c->self);
 			continue;
 		}
 
Index: channels.h
===================================================================
RCS file: /cvs/openssh_cvs/channels.h,v
retrieving revision 1.41
diff -u -r1.41 channels.h
--- channels.h	2001/10/10 05:14:39	1.41
+++ channels.h	2001/10/10 20:26:54
@@ -214,7 +214,7 @@
 
 /* channel close */
 
-int	 chan_is_dead(Channel *);
+int	 chan_is_dead(Channel *, int);
 void	 chan_mark_dead(Channel *);
 void 	 chan_init_iostates(Channel *);
 void	 chan_init(void);
Index: clientloop.c
===================================================================
RCS file: /cvs/openssh_cvs/clientloop.c,v
retrieving revision 1.65
diff -u -r1.65 clientloop.c
--- clientloop.c	2001/09/18 05:51:14	1.65
+++ clientloop.c	2001/10/10 20:26:57
@@ -753,6 +753,7 @@
 	if (id != session_ident)
 		error("client_channel_closed: id %d != session_ident %d",
 		    id, session_ident);
+	channel_cancel_cleanup(id);
 	session_closed = 1;
 	if (in_raw_mode())
 		leave_raw_mode();
Index: nchan.c
===================================================================
RCS file: /cvs/openssh_cvs/nchan.c,v
retrieving revision 1.28
diff -u -r1.28 nchan.c
--- nchan.c	2001/09/20 19:33:33	1.28
+++ nchan.c	2001/10/10 20:26:58
@@ -432,7 +432,7 @@
 }
 
 int
-chan_is_dead(Channel *c)
+chan_is_dead(Channel *c, int send)
 {
 	if (c->type == SSH_CHANNEL_ZOMBIE) {
 		debug("channel %d: zombie", c->self);
@@ -461,7 +461,14 @@
 		       "read": "write");
 	} else {
 		if (!(c->flags & CHAN_CLOSE_SENT)) {
-			chan_send_close2(c);
+			if (send) {
+				chan_send_close2(c);
+			} else {
+				if (c->flags & CHAN_CLOSE_RCVD) {
+					debug("channel %d: XXX almost dead", c->self);
+					return 1;
+				}
+			}
 		}
 		if ((c->flags & CHAN_CLOSE_SENT) &&
 		    (c->flags & CHAN_CLOSE_RCVD)) {
Index: serverloop.c
===================================================================
RCS file: /cvs/openssh_cvs/serverloop.c,v
retrieving revision 1.80
diff -u -r1.80 serverloop.c
--- serverloop.c	2001/10/10 05:14:39	1.80
+++ serverloop.c	2001/10/10 20:26:59
@@ -208,9 +208,6 @@
 		max_time_milliseconds = options.client_alive_interval * 1000;
 	}
 
-	/* When select fails we restart from here. */
-retry_select:
-
 	/* Allocate and update select() masks for channel descriptors. */
 	channel_prepare_select(readsetp, writesetp, maxfdp, nallocp, 0);
 
@@ -275,10 +272,11 @@
 	ret = select((*maxfdp)+1, *readsetp, *writesetp, NULL, tvp);
 
 	if (ret == -1) {
+		memset(*readsetp, 0, *maxfdp);
+		memset(*writesetp, 0, *maxfdp);
 		if (errno != EINTR)
 			error("select: %.100s", strerror(errno));
-		else
-			goto retry_select;
+		debug("XXX select: %.100s", strerror(errno));
 	}
 	if (ret == 0 && client_alive_scheduled)
 		client_alive_check();
@@ -668,13 +666,30 @@
 	/* NOTREACHED */
 }
 
+static void
+collect_children(void)
+{
+	pid_t pid;
+	sigset_t oset, nset;
+	int status;
+
+	/* block SIGCHLD while we check for dead children */
+	sigemptyset(&nset);
+	sigaddset(&nset, SIGCHLD);
+	sigprocmask(SIG_BLOCK, &nset, &oset);
+	if (child_terminated) {
+		while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
+			session_close_by_pid(pid, status);
+		child_terminated = 0;
+	}
+	sigprocmask(SIG_SETMASK, &oset, NULL);
+}
+
 void
 server_loop2(Authctxt *authctxt)
 {
 	fd_set *readset = NULL, *writeset = NULL;
-	int rekeying = 0, max_fd, status, nalloc = 0;
-	pid_t pid;
-	sigset_t oset, nset;
+	int rekeying = 0, max_fd, nalloc = 0;
 
 	debug("Entering interactive session for SSH2.");
 
@@ -698,16 +713,7 @@
 		wait_until_can_do_something(&readset, &writeset, &max_fd,
 		    &nalloc, 0);
 
-		/* block SIGCHLD while we check for dead children */
-		sigemptyset(&nset);
-		sigaddset(&nset, SIGCHLD);
-		sigprocmask(SIG_BLOCK, &nset, &oset);
-		if (child_terminated) {
-			while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
-				session_close_by_pid(pid, status);
-			child_terminated = 0;
-		}
-		sigprocmask(SIG_SETMASK, &oset, NULL);
+		collect_children();
 		if (!rekeying)
 			channel_after_select(readset, writeset);
 		process_input(readset);
@@ -715,6 +721,8 @@
 			break;
 		process_output(writeset);
 	}
+	collect_children();
+
 	if (readset)
 		xfree(readset);
 	if (writeset)
@@ -722,11 +730,6 @@
 
 	/* free all channels, no more reads and writes */
 	channel_free_all();
-
-	/* collect remaining dead children, XXX not necessary? */
-	mysignal(SIGCHLD, SIG_DFL);
-	while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
-		session_close_by_pid(pid, status);
 
 	/* close remaining sessions, e.g remove wtmp entries */
 	session_close_all();
Index: session.c
===================================================================
RCS file: /cvs/openssh_cvs/session.c,v
retrieving revision 1.152
diff -u -r1.152 session.c
--- session.c	2001/10/10 05:14:39	1.152
+++ session.c	2001/10/10 20:27:03
@@ -1961,18 +1961,15 @@
 		debug("session_close_by_channel: no session for channel %d", id);
 		return;
 	}
-	/* disconnect channel */
-	channel_cancel_cleanup(s->chanid);
-	s->chanid = -1;
-
-	debug("session_close_by_channel: channel %d kill %d", id, s->pid);
+	debug("session_close_by_channel: channel %d child %d", id, s->pid);
 	if (s->pid != 0) {
-		/* notify child */
-		if (kill(s->pid, SIGHUP) < 0)
-			error("session_close_by_channel: kill %d: %s",
-			    s->pid, strerror(errno));
+		debug("session_close_by_channel: channel %d: has child", id);
+		return;
 	}
+	channel_cancel_cleanup(s->chanid);
+	s->chanid = -1;
 	session_close(s);
+	return;
 }
 
 void



More information about the openssh-unix-dev mailing list