Merge branch 'dev_minor' into oss_fuzz_tests

Author: rapier1

channels.c

@@ -534,9 +534,16 @@ channel_new(struct ssh *ssh, char *ctype, int type, int rfd, int wfd, int efd,
 	    (c->output = sshbuf_new()) == NULL ||
 	    (c->extended = sshbuf_new()) == NULL)
 		fatal_f("sshbuf_new failed");
+
+	/* these buffers are important in terms of tracking channel
+	 * buffer usage so label and type them with descriptive names */
 	sshbuf_relabel(c->input, "channel input");
+	sshbuf_type(c->input, BUF_CHANNEL_INPUT);
 	sshbuf_relabel(c->output, "channel output");
+	sshbuf_type(c->output, BUF_CHANNEL_OUTPUT);
 	sshbuf_relabel(c->extended, "channel extended");
+	sshbuf_type(c->extended, BUF_CHANNEL_EXTENDED);
+
 	if ((r = sshbuf_set_max_size(c->input, CHAN_INPUT_MAX)) != 0)
 		fatal_fr(r, "sshbuf_set_max_size");
 	c->ostate = CHAN_OUTPUT_OPEN;

packet.c

@@ -257,11 +257,15 @@ ssh_alloc_session_state(void)
 	    (state->incoming_packet = sshbuf_new()) == NULL)
 		goto fail;
 	/* these buffers are important in terms of tracking buffer usage
-	 * so we explicitly label them with descriptive names */
+	 * so we explicitly label and type them with descriptive names */
 	sshbuf_relabel(state->input, "input");
+	sshbuf_type(state->input, "BUF_PACKET_INPUT");
 	sshbuf_relabel(state->incoming_packet, "inpacket");
+	sshbuf_type(state->incoming_packet, "BUF_PACKET_INCOMING");
 	sshbuf_relabel(state->output, "output");
+	sshbuf_type(state->output, "BUF_PACKET_OUTPUT");
 	sshbuf_relabel(state->outgoing_packet, "outpacket");
+	sshbuf_type(state->outgoing_packet, "BUF_PACKET_OUTGOING");
 
 	TAILQ_INIT(&state->outgoing);
 	TAILQ_INIT(&ssh->private_keys);

sshbuf.c

@@ -42,42 +42,37 @@
 # define SSHBUF_TELL(what)
 #endif
 
-#ifdef SSHBUF_DEBUG
-# define SSHBUF_TELL(what) do { \
-		printf("%s:%d %s: %s size %zu alloc %zu off %zu max %zu\n", \
-		    __FILE__, __LINE__, __func__, what, \
-		    buf->size, buf->alloc, buf->off, buf->max_size); \
-		fflush(stdout); \
-	} while (0)
-#else
-# define SSHBUF_TELL(what)
-#endif
-
 struct sshbuf {
 	u_char *d;		/* Data */
 	const u_char *cd;	/* Const data */
 	size_t off;		/* First available byte is buf->d + buf->off */
 	size_t size;		/* Last byte is buf->d + buf->size - 1 */
 	size_t max_size;	/* Maximum size of buffer */
-        size_t window_max;      /* channel window max */
-        size_t alloc;		/* Total bytes allocated to buf->d */
+	size_t alloc;		/* Total bytes allocated to buf->d */
 	int readonly;		/* Refers to external, const data */
 	u_int refcount;		/* Tracks self and number of child buffers */
 	struct sshbuf *parent;	/* If child, pointer to parent */
 	char label[MAX_LABEL_LEN];   /* String for buffer label - debugging use */
-	struct timeval buf_ts; /* creation time of buffer */
+	int type;               /* type of buffer enum (sshbuf_types)*/
 };
 
+/* update the label string for a given sshbuf. Useful
+ * for debugging */
 void
 sshbuf_relabel(struct sshbuf *buf, const char *label)
 {
 	if (label != NULL)
 		strncpy(buf->label, label, MAX_LABEL_LEN-1);
 }
 
-float time_diff(struct timeval *start, struct timeval *end)
+/* set the type (from enum sshbuf_type) of the given sshbuf.
+ * The purpose is to allow different classes of buffers to
+ * follow different code paths if necessary */
+void
+sshbuf_type(struct sshbuf *buf, int type)
 {
-	return (end->tv_sec - start->tv_sec) + 1e-6*(end->tv_usec - start->tv_usec);
+	if (type < BUF_MAX_TYPE)
+		buf->type = type;
 }
 
 static inline int
@@ -249,7 +244,7 @@ sshbuf_reset(struct sshbuf *buf)
 size_t
 sshbuf_max_size(const struct sshbuf *buf)
 {
-	return buf->max_size / 2;
+	return buf->max_size;
 }
 
 size_t
@@ -271,18 +266,13 @@ sshbuf_refcount(const struct sshbuf *buf)
 }
 
 int
-sshbuf_set_max_size(struct sshbuf *buf, size_t requested_size)
+sshbuf_set_max_size(struct sshbuf *buf, size_t max_size)
 {
-	size_t rlen, max_size = requested_size * 2;
+	size_t rlen;
 	u_char *dp;
 	int r;
 
-	/*
-	 * Note that we set the actual allocation limit to be 2x the requested
-	 * size. This is to avoid some pathological compaction behaviour later
-	 * when a buffer is at capacity and has small drains/fills on it.
-	 */
-	SSHBUF_DBG(("set max buf = %p requested = %zu", buf, requested_size));
+	SSHBUF_DBG(("set max buf = %p len = %zu", buf, max_size));
 	if ((r = sshbuf_check_sanity(buf)) != 0)
 		return r;
 	if (max_size == buf->max_size)
@@ -291,17 +281,9 @@ sshbuf_set_max_size(struct sshbuf *buf, size_t requested_size)
 		return SSH_ERR_BUFFER_READ_ONLY;
 	if (max_size > SSHBUF_SIZE_MAX)
 		return SSH_ERR_NO_BUFFER_SPACE;
-	/*
-	 * Always pack as it makes everything that follows easier.
-	 * Potentially expensive, but this should seldom be called on buffers
-	 * that already contain data.
-	 */
-	sshbuf_maybe_pack(buf, 1);
-	/* Refuse setting a maximum below current amount of data in buffer */
-	if (requested_size < buf->size)
-		return SSH_ERR_NO_BUFFER_SPACE;
-	/* Shrink alloc if the existing allocation is larger than requested */
-	if (requested_size < buf->alloc) {
+	/* pack and realloc if necessary */
+	sshbuf_maybe_pack(buf, max_size < buf->size);
+	if (max_size < buf->alloc && max_size > buf->size) {
 		if (buf->size < SSHBUF_SIZE_INIT)
 			rlen = SSHBUF_SIZE_INIT;
 		else
@@ -315,6 +297,8 @@ sshbuf_set_max_size(struct sshbuf *buf, size_t requested_size)
 		buf->alloc = rlen;
 	}
 	SSHBUF_TELL("new-max");
+	if (max_size < buf->alloc)
+		return SSH_ERR_NO_BUFFER_SPACE;
 	buf->max_size = max_size;
 	return 0;
 }
@@ -332,7 +316,18 @@ sshbuf_avail(const struct sshbuf *buf)
 {
 	if (sshbuf_check_sanity(buf) != 0 || buf->readonly || buf->refcount > 1)
 		return 0;
-	return (buf->max_size / 2) - (buf->size - buf->off);
+	/* we need to reserve a small amount of overhead on the input buffer
+	 * or we can enter into a pathological state during bulk
+	 * data transfers. We use a fraction of the max size as we want it to scale
+	 * with the size of the input buffer. If we do it for all of the buffers
+	 * we fail the regression unit tests. This seems like a reasonable
+	 * solution. Of course, I still need to figure out *why* this is
+	 * happening and come up with an actual fix. TODO
+	 * cjr 4/19/2024 */
+	if (buf->type == BUF_CHANNEL_INPUT)
+		return buf->max_size / 1.05 - (buf->size - buf->off);
+	else
+		return buf->max_size - (buf->size - buf->off);
 }
 
 const u_char *
@@ -361,19 +356,12 @@ sshbuf_check_reserve(const struct sshbuf *buf, size_t len)
 	if (buf->readonly || buf->refcount > 1)
 		return SSH_ERR_BUFFER_READ_ONLY;
 	SSHBUF_TELL("check");
-	/* Check that len is reasonable and that max size + available < len */
-	if (len > (buf->max_size / 2) ||
-	    (buf->max_size / 2) - len < buf->size - buf->off)
+	/* Check that len is reasonable and that max_size + available < len */
+	if (len > buf->max_size || buf->max_size - len < buf->size - buf->off)
 		return SSH_ERR_NO_BUFFER_SPACE;
 	return 0;
 }
 
-void
-sshbuf_set_window_max(struct sshbuf *buf, size_t len)
-{
-	buf->window_max = len;
-}
-
 int
 sshbuf_allocate(struct sshbuf *buf, size_t len)
 {
@@ -403,38 +391,32 @@ sshbuf_allocate(struct sshbuf *buf, size_t len)
 	 * slowly. It's knows that it needs to grow but it only does so 32K
 	 * at a time. This means a lot of calls to realloc and memcpy which
 	 * kills performance until the buffer reaches some maximum size.
-	 * so we explicitly test for a buffer that's trying to grow and
-	 * if it is then we push the growth to whatever the adjusted value of
-	 * local_window_max happens to be. This significantly reduces overhead
+	 * So we explicitly test for a buffer that's trying to grow and
+	 * if it is then we push the growth by 4MB at a time. This can result in
+	 * the buffer being over allocated (in terms of actual needs) but the
+	 * process is fast. This significantly reduces overhead
 	 * and improves performance. In this case we look for a buffer that is trying
 	 * to grow larger than BUF_WATERSHED (256*1024 taken from PACKET_MAX_SIZE)
-	 * and where the local_window_max isn't zero (which is usally in the Channels
-	 * struct but we copied it into the shhbuf as window_max). If it is zero or
-	 * the buffer is smaller than BUF_WATERSHED we just use the
-	 * normal value for need. We also don't want to grow the buffer past
-	 * what we need (the size of window_max) so if the current allocation (in
-	 * buf->alloc) is greater than window_max we skip it.
-	 *
-	 * Turns out the extra functions on the following conditional aren't needed
-	 * -cjr 04/06/23
+	 * and explcitly check that the buffer is being used for inbound outbound
+	 * channel buffering.
+	 * Updated for 18.4.1 -cjr 04/20/24
 	 */
-	if (rlen > BUF_WATERSHED) {
-		/* debug_f ("Prior: label: %s, %p, rlen is %zu need is %zu win_max is %zu max_size is %zu",
-		   buf->label, buf, rlen, need, buf->window_max, buf->max_size); */
+	if (rlen > BUF_WATERSHED && (buf->type == BUF_CHANNEL_OUTPUT || buf->type == BUF_CHANNEL_INPUT)) {
+		/* debug_f ("Prior: label: %s, %p, rlen is %zu need is %zu max_size is %zu",
+		   buf->label, buf, rlen, need, buf->max_size); */
 		/* easiest thing to do is grow the nuffer by 4MB each time. It might end
 		 * up being somewhat overallocated but works quickly */
 		need = (4*1024*1024);
 		rlen = ROUNDUP(buf->alloc + need, SSHBUF_SIZE_INC);
-		/* debug_f ("Post: label: %s, %p, rlen is %zu need is %zu win_max is %zu max_size is %zu", */
-		/* 	 buf->label, buf, rlen, need, buf->window_max, buf->max_size); */
+		/* debug_f ("Post: label: %s, %p, rlen is %zu need is %zu max_size is %zu", */
+		/* 	 buf->label, buf, rlen, need, buf->max_size); */
 	}
 	SSHBUF_DBG(("need %zu initial rlen %zu", need, rlen));
 
 	/* rlen might be above the max allocation */
-	if (rlen > buf->max_size) {
+	if (rlen > buf->max_size)
 		rlen = buf->max_size;
-		/* debug_f("set rlen to %zu", buf->max_size);*/
-	}
+
 	SSHBUF_DBG(("adjusted rlen %zu", rlen));
 	if ((dp = recallocarray(buf->d, buf->alloc, rlen, 1)) == NULL) {
 		SSHBUF_DBG(("realloc fail"));
@@ -484,9 +466,8 @@ sshbuf_consume(struct sshbuf *buf, size_t len)
 		return SSH_ERR_MESSAGE_INCOMPLETE;
 	buf->off += len;
 	/* deal with empty buffer */
-	if (buf->off == buf->size) {
+	if (buf->off == buf->size)
 		buf->off = buf->size = 0;
-	}
 	SSHBUF_TELL("done");
 	return 0;
 }

sshbuf.h

@@ -28,7 +28,7 @@
 # endif /* OPENSSL_HAS_ECC */
 #endif /* WITH_OPENSSL */
 
-#define SSHBUF_SIZE_MAX		0xFFFFFFF	/* Hard maximum size 256MB */
+#define SSHBUF_SIZE_MAX		0x8000000	/* Hard maximum size 128MB */
 #define SSHBUF_REFS_MAX		0x100000	/* Max child buffers */
 #define SSHBUF_MAX_BIGNUM	(16384 / 8)	/* Max bignum *bytes* */
 #define SSHBUF_MAX_ECPOINT	((528 * 2 / 8) + 1) /* Max EC point *bytes* */
@@ -37,11 +37,22 @@
 
 struct sshbuf;
 
+enum buffer_types {
+	BUF_CHANNEL_OUTPUT,
+	BUF_CHANNEL_INPUT,
+	BUF_CHANNEL_EXTENDED,
+	BUF_PACKET_INPUT,
+	BUF_PACKET_INCOMING,
+	BUF_PACKET_OUTPUT,
+	BUF_PACKET_OUTGOING,
+	BUF_MAX_TYPE
+};
+
 /*
  * Create a new sshbuf buffer.
  * Returns pointer to buffer on success, or NULL on allocation failure.
  */
-/* struct sshbuf *sshbuf_new(void); */
+/* struct sshbuf *sshbuf_new(void);*/
 
 /*
  * Create a new labeled sshbuf buffer.
@@ -54,6 +65,13 @@ struct sshbuf *sshbuf_new_label(const char *);
  */
 void sshbuf_relabel(struct sshbuf *, const char *);
 
+/*
+ * assign a type (from the buffer_types enum) to
+ * the buffer. Used to quickly identify the purpose of
+ * the buffer.
+ */
+void sshbuf_type(struct sshbuf *, int);
+
 /*
  * Create a new, read-only sshbuf buffer from existing data.
  * Returns pointer to buffer on success, or NULL on allocation failure.