Attachment 3182 Details for Bug 2863 – close efd when shutting down read/write

[patch] close efd when shutting down read/write

bz2863.diff (text/plain), 6.29 KB, created by Damien Miller on 2018-10-03 18:30:03 AEST

(hide)

Description:

Filename:

MIME Type:

Creator: Damien Miller

Created: 2018-10-03 18:30:03 AEST

Size: 6.29 KB

patch

obsolete

>commit 10e3f02c0a4c697c7c5857724e2b41d9e923c8a0
>Author: Damien Miller <djm@mindrot.org>
>Date:   Wed Oct 3 18:27:12 2018 +1000
>
>    close efd when closing read/write
>    
>    add some more useful info to debug messages too
>
>diff --git a/channels.c b/channels.c
>index 9482e9e..a849130 100644
>--- a/channels.c
>+++ b/channels.c
>@@ -789,6 +789,25 @@ channel_find_open(struct ssh *ssh)
> 	return -1;
> }
> 
>+/* Returns the state of the channel's extended usage flag */
>+const char *
>+channel_format_extended_usage(const Channel *c)
>+{
>+	if (c->efd == -1)
>+		return "closed";
>+
>+	switch (c->extended_usage) {
>+	case CHAN_EXTENDED_WRITE:
>+		return "write";
>+	case CHAN_EXTENDED_READ:
>+		return "read";
>+	case CHAN_EXTENDED_IGNORE:
>+		return "ignore";
>+	default:
>+		return "UNKNOWN";
>+	}
>+}
>+
> /*
>  * Returns a message describing the currently open forwarded connections,
>  * suitable for sending to the client.  The message contains crlf pairs for
>@@ -835,13 +854,16 @@ channel_open_message(struct ssh *ssh)
> 		case SSH_CHANNEL_MUX_PROXY:
> 		case SSH_CHANNEL_MUX_CLIENT:
> 			if ((r = sshbuf_putf(buf, "  #%d %.300s "
>-			    "(t%d %s%u i%u/%zu o%u/%zu fd %d/%d cc %d)\r\n",
>+			    "(t%d %s%u i%u/%zu o%u/%zu "
>+			    "fd %d/%d/%d [%s] sock %d cc %d)\r\n",
> 			    c->self, c->remote_name,
> 			    c->type,
> 			    c->have_remote_id ? "r" : "nr", c->remote_id,
> 			    c->istate, sshbuf_len(c->input),
> 			    c->ostate, sshbuf_len(c->output),
>-			    c->rfd, c->wfd, c->ctl_chan)) != 0)
>+			    c->rfd, c->wfd, c->efd,
>+			    channel_format_extended_usage(c),
>+			    c->sock, c->ctl_chan)) != 0)
> 				fatal("%s: sshbuf_putf: %s",
> 				    __func__, ssh_err(r));
> 			continue;
>@@ -2321,6 +2343,7 @@ channel_garbage_collect(struct ssh *ssh, Channel *c)
> 	if (c->detach_user != NULL) {
> 		if (!chan_is_dead(ssh, c, c->detach_close))
> 			return;
>+
> 		debug2("channel %d: gc: notify user", c->self);
> 		c->detach_user(ssh, c->self, NULL);
> 		/* if we still have a callback */
>diff --git a/channels.h b/channels.h
>index 8bee548..3c8c69c 100644
>--- a/channels.h
>+++ b/channels.h
>@@ -282,6 +282,7 @@ void     channel_output_poll(struct ssh *);
> int      channel_not_very_much_buffered_data(struct ssh *);
> void     channel_close_all(struct ssh *);
> int      channel_still_open(struct ssh *);
>+const char *channel_format_extended_usage(const Channel *);
> char	*channel_open_message(struct ssh *);
> int	 channel_find_open(struct ssh *);
> 
>diff --git a/nchan.c b/nchan.c
>index db68643..763921d 100644
>--- a/nchan.c
>+++ b/nchan.c
>@@ -371,17 +371,30 @@ chan_shutdown_write(struct ssh *ssh, Channel *c)
> 	if (c->type == SSH_CHANNEL_LARVAL)
> 		return;
> 	/* shutdown failure is allowed if write failed already */
>-	debug2("channel %d: close_write", c->self);
>+	debug2("channel %d: %s (i%d o%d sock %d wfd %d efd %d [%s])",
>+	    c->self, __func__, c->istate, c->ostate, c->sock, c->wfd, c->efd,
>+	    channel_format_extended_usage(c));
> 	if (c->sock != -1) {
>-		if (shutdown(c->sock, SHUT_WR) < 0)
>-			debug2("channel %d: chan_shutdown_write: "
>-			    "shutdown() failed for fd %d: %.100s",
>-			    c->self, c->sock, strerror(errno));
>+		if (shutdown(c->sock, SHUT_WR) < 0) {
>+			debug2("channel %d: %s: shutdown() failed for "
>+			    "fd %d [i%d o%d]: %.100s", c->self, __func__,
>+			    c->sock, c->istate, c->ostate,
>+			    strerror(errno));
>+		}
> 	} else {
>-		if (channel_close_fd(ssh, &c->wfd) < 0)
>-			logit("channel %d: chan_shutdown_write: "
>-			    "close() failed for fd %d: %.100s",
>-			    c->self, c->wfd, strerror(errno));
>+		if (channel_close_fd(ssh, &c->wfd) < 0) {
>+			logit("channel %d: %s: close() failed for "
>+			    "fd %d [i%d o%d]: %.100s",
>+			    c->self, __func__, c->wfd, c->istate, c->ostate,
>+			    strerror(errno));
>+		}
>+		if (c->extended_usage == CHAN_EXTENDED_WRITE &&
>+		    c->efd != -1 && channel_close_fd(ssh, &c->efd) < 0) {
>+			logit("channel %d: %s: close() failed for "
>+			    "extended fd %d [i%d o%d]: %.100s",
>+			    c->self, __func__, c->efd, c->istate, c->ostate,
>+			    strerror(errno));
>+		}
> 	}
> }
> 
>@@ -390,17 +403,29 @@ chan_shutdown_read(struct ssh *ssh, Channel *c)
> {
> 	if (c->type == SSH_CHANNEL_LARVAL)
> 		return;
>-	debug2("channel %d: close_read", c->self);
>+	debug2("channel %d: %s (i%d o%d sock %d wfd %d efd %d [%s])",
>+	    c->self, __func__, c->istate, c->ostate, c->sock, c->rfd, c->efd,
>+	    channel_format_extended_usage(c));
> 	if (c->sock != -1) {
>-		if (shutdown(c->sock, SHUT_RD) < 0)
>-			error("channel %d: chan_shutdown_read: "
>-			    "shutdown() failed for fd %d [i%d o%d]: %.100s",
>-			    c->self, c->sock, c->istate, c->ostate,
>+		if (shutdown(c->sock, SHUT_RD) < 0) {
>+			error("channel %d: %s: shutdown() failed for "
>+			    "fd %d [i%d o%d]: %.100s",
>+			    c->self, __func__, c->sock, c->istate, c->ostate,
> 			    strerror(errno));
>+		}
> 	} else {
>-		if (channel_close_fd(ssh, &c->rfd) < 0)
>-			logit("channel %d: chan_shutdown_read: "
>-			    "close() failed for fd %d: %.100s",
>-			    c->self, c->rfd, strerror(errno));
>+		if (channel_close_fd(ssh, &c->rfd) < 0) {
>+			logit("channel %d: %s: close() failed for "
>+			    "fd %d [i%d o%d]: %.100s",
>+			    c->self, __func__, c->rfd, c->istate, c->ostate,
>+			    strerror(errno));
>+		}
>+		if (c->extended_usage == CHAN_EXTENDED_READ &&
>+		    c->efd != -1 && channel_close_fd(ssh, &c->efd) < 0) {
>+			logit("channel %d: %s: close() failed for "
>+			    "extended fd %d [i%d o%d]: %.100s",
>+			    c->self, __func__, c->efd, c->istate, c->ostate,
>+			    strerror(errno));
>+		}
> 	}
> }
>diff --git a/session.c b/session.c
>index 0b1d8b0..382f84a 100644
>--- a/session.c
>+++ b/session.c
>@@ -1915,13 +1915,13 @@ void
> session_pty_cleanup2(Session *s)
> {
> 	if (s == NULL) {
>-		error("session_pty_cleanup: no session");
>+		error("%s: no session", __func__);
> 		return;
> 	}
> 	if (s->ttyfd == -1)
> 		return;
> 
>-	debug("session_pty_cleanup: session %d release %s", s->self, s->tty);
>+	debug("%s: session %d release %s", __func__, s->self, s->tty);
> 
> 	/* Record that the user has logged out. */
> 	if (s->pid != 0)
>@@ -2128,7 +2128,8 @@ session_close_by_channel(struct ssh *ssh, int id, void *arg)
> 	}
> 	debug("%s: channel %d child %ld", __func__, id, (long)s->pid);
> 	if (s->pid != 0) {
>-		debug("%s: channel %d: has child", __func__, id);
>+		debug("%s: channel %d: has child, ttyfd %d",
>+		    __func__, id, s->ttyfd);
> 		/*
> 		 * delay detach of session, but release pty, since
> 		 * the fd's to the child are already closed

Actions: View | Diff

Attachments on bug 2863: 3182 | 3185