On Wed, 10 Nov 2021, rapier wrote:
> This patch seems to be faster but I'm running into a weird issue.
>
> I have your patch running on the server iztli10 on port 2205. If I run this
> command:
> "dd if=/dev/urandom bs=1024 count=5000 | /opt/ssh/stock-8.7/bin/ssh
> -caes256-ctr -p 2205 iztli10 'cat > /dev/null' > foo"
>
> I end up with 2097152 bytes of nulls in 'foo'. I see this when running a stock
> 8.7 client or a patched client. The patch didn't apply cleanly to 8.7 but I
> might not have integrated it properly. What version of ssh did you generate
> this patch against?
that patch had a bug that I just found yesterday. Try this one:
diff --git a/channels.c b/channels.c
index 4903ad1..d6b2024 100644
--- a/channels.c
+++ b/channels.c
@@ -1928,16 +1928,41 @@ channel_handle_rfd(struct ssh *ssh, Channel *c,
char buf[CHAN_RBUF];
ssize_t len;
int r;
+ size_t avail, rlen, maxlen;
if (c->rfd == -1 || !FD_ISSET(c->rfd, readset))
return 1;
+ /*
+ * For "simple" channels (i.e. not datagram or filtered), try to
+ * read up to a complete remote window of data directly to the
+ * channel buffer.
+ */
+ if (c->input_filter == NULL && !c->datagram) {
+ if (sshbuf_len(c->input) >= c->remote_window ||
+ (avail = sshbuf_avail(c->input)) == 0)
+ return 1; /* shouldn't happen */
+ maxlen = c->remote_window - sshbuf_len(c->input);
+ if (maxlen > avail)
+ maxlen = avail;
+ if (maxlen > CHANNEL_MAX_READ)
+ maxlen = CHANNEL_MAX_READ;
+ if ((r = sshbuf_read(c->rfd, c->input, maxlen, &rlen)) != 0) {
+ debug2("channel %d: read failed rfd %d maxlen %zu: %s",
+ c->self, c->rfd, maxlen, ssh_err(r));
+ goto rfail;
+ }
+ return 1;
+ }
+
len = read(c->rfd, buf, sizeof(buf));
if (len == -1 && (errno == EINTR || errno == EAGAIN))
return 1;
if (len <= 0) {
- debug2("channel %d: read<=0 rfd %d len %zd",
- c->self, c->rfd, len);
+ debug2("channel %d: read<=0 rfd %d len %zd: %s",
+ c->self, c->rfd, len,
+ len == 0 ? "closed" : strerror(errno));
+ rfail:
if (c->type != SSH_CHANNEL_OPEN) {
debug2("channel %d: not open", c->self);
chan_mark_dead(ssh, c);
@@ -1955,8 +1980,7 @@ channel_handle_rfd(struct ssh *ssh, Channel *c,
} else if (c->datagram) {
if ((r = sshbuf_put_string(c->input, buf, len)) != 0)
fatal_fr(r, "channel %i: put datagram", c->self);
- } else if ((r = sshbuf_put(c->input, buf, len)) != 0)
- fatal_fr(r, "channel %i: put data", c->self);
+ }
return 1;
}
diff --git a/channels.h b/channels.h
index 633adc3..db90c18 100644
--- a/channels.h
+++ b/channels.h
@@ -231,6 +231,9 @@ struct Channel {
/* Read buffer size */
#define CHAN_RBUF (16*1024)
+/* Maximum size for direct reads to buffers */
+#define CHANNEL_MAX_READ (CHAN_SES_WINDOW_DEFAULT*2)
+
/* Maximum channel input buffer size */
#define CHAN_INPUT_MAX (16*1024*1024)
diff --git a/sshbuf-io.c b/sshbuf-io.c
index 966f820..d41282d 100644
--- a/sshbuf-io.c
+++ b/sshbuf-io.c
@@ -113,3 +113,39 @@ sshbuf_write_file(const char *path, struct sshbuf *buf)
return 0;
}
+int
+sshbuf_read(int fd, struct sshbuf *buf, size_t maxlen, size_t *rlen)
+{
+ int r, oerrno;
+ size_t adjust;
+ ssize_t rr;
+ u_char *d;
+
+ if (rlen != NULL)
+ *rlen = 0;
+ if ((r = sshbuf_reserve(buf, maxlen, &d)) != 0)
+ return r;
+ rr = read(fd, d, maxlen);
+ oerrno = errno;
+
+ /* Adjust the buffer to include only what was actually read */
+ if ((adjust = maxlen - (rr > 0 ? rr : 0)) != 0) {
+ if ((r = sshbuf_consume_end(buf, adjust)) != 0) {
+ /* avoid returning uninitialised data to caller */
+ memset(d + rr, '\0', adjust);
+ return SSH_ERR_INTERNAL_ERROR; /* shouldn't happen */
+ }
+ }
+ if (rr < 0) {
+ errno = oerrno;
+ return SSH_ERR_SYSTEM_ERROR;
+ } else if (rr == 0) {
+ errno = EPIPE;
+ return SSH_ERR_SYSTEM_ERROR;
+ }
+ /* success */
+ if (rlen != NULL)
+ *rlen = (size_t)rr;
+ return 0;
+}
+
diff --git a/sshbuf.h b/sshbuf.h
index 2b77d15..1ee9266 100644
--- a/sshbuf.h
+++ b/sshbuf.h
@@ -310,6 +310,10 @@ int sshbuf_load_file(const char *, struct sshbuf **)
int sshbuf_write_file(const char *path, struct sshbuf *buf)
__attribute__((__nonnull__ (2)));
+/* Read up to maxlen bytes from a fd directly to a buffer */
+int sshbuf_read(int, struct sshbuf *, size_t, size_t *)
+ __attribute__((__nonnull__ (2)));
+
/* Macros for decoding/encoding integers */
#define PEEK_U64(p) \
(((u_int64_t)(((const u_char *)(p))[0]) << 56) | \
_______________________________________________
openssh-unix-dev mailing list
openssh-unix-dev@xxxxxxxxxxx
https://lists.mindrot.org/mailman/listinfo/openssh-unix-dev
