From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 11 May 2023 14:52:17 +0000 (+0200)
Subject: BUG/MINOR: mux-quic: properly handle buf alloc failure
X-Git-Tag: v2.8-dev12~41
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0abde9dee69fe151f5f181a34e0782ef840abe53;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mux-quic: properly handle buf alloc failure

A convenience function qc_get_buf() is implemented to centralize buffer
allocation on MUX and H3 layers. However, allocation failure was not
handled properly with a BUG_ON() statement.

Replace this by proper error management. On emission, streams is
temporarily skip over until the next qc_send() invocation. On reception,
H3 uses this function for HTX conversion; on alloc failure the
connection will be closed with QUIC internal error code.

This must be backported up to 2.6.
---

diff --git a/src/h3.c b/src/h3.c
index d41a20c21d..99c2d76474 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -453,7 +453,12 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
 		goto out;
 	}
 
-	qc_get_buf(qcs, &htx_buf);
+	if (!qc_get_buf(qcs, &htx_buf)) {
+		TRACE_ERROR("HTX buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+		h3c->err = H3_INTERNAL_ERROR;
+		len = -1;
+		goto out;
+	}
 	BUG_ON(!b_size(&htx_buf)); /* TODO */
 	htx = htx_from_buf(&htx_buf);
 
@@ -746,7 +751,12 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
 		goto out;
 	}
 
-	qc_get_buf(qcs, &htx_buf);
+	if (!qc_get_buf(qcs, &htx_buf)) {
+		TRACE_ERROR("HTX buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+		h3c->err = H3_INTERNAL_ERROR;
+		len = -1;
+		goto out;
+	}
 	BUG_ON(!b_size(&htx_buf)); /* TODO */
 	htx = htx_from_buf(&htx_buf);
 
@@ -847,6 +857,8 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
 static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
                               uint64_t len, char fin)
 {
+	struct h3s *h3s = qcs->ctx;
+	struct h3c *h3c = h3s->h3c;
 	struct buffer *appbuf;
 	struct htx *htx = NULL;
 	size_t htx_sent = 0;
@@ -855,8 +867,13 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
 
 	TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
 
-	appbuf = qc_get_buf(qcs, &qcs->rx.app_buf);
-	BUG_ON(!appbuf);
+	if (!(appbuf = qc_get_buf(qcs, &qcs->rx.app_buf))) {
+		TRACE_ERROR("data buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
+		h3c->err = H3_INTERNAL_ERROR;
+		len = -1;
+		goto out;
+	}
+
 	htx = htx_from_buf(appbuf);
 
 	if (len > b_data(buf)) {
@@ -900,7 +917,8 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
 		htx->flags |= HTX_FL_EOM;
 
  out:
-	htx_to_buf(htx, appbuf);
+	if (appbuf)
+		htx_to_buf(htx, appbuf);
 
 	TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
 	return htx_sent;
@@ -1040,8 +1058,10 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
 		struct htx *htx;
 
 		TRACE_PROTO("received FIN without data", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-		appbuf = qc_get_buf(qcs, &qcs->rx.app_buf);
-		BUG_ON(!appbuf);
+		if (!(appbuf = qc_get_buf(qcs, &qcs->rx.app_buf))) {
+			TRACE_ERROR("data buffer alloc failure", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+			h3c->err = H3_INTERNAL_ERROR;
+		}
 
 		htx = htx_from_buf(appbuf);
 		if (!htx_set_eom(htx)) {
diff --git a/src/mux_quic.c b/src/mux_quic.c
index 01698f05d0..294a0d7b82 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -417,11 +417,13 @@ static int qcs_is_close_remote(struct qcs *qcs)
 	return qcs->st == QC_SS_HREM || qcs->st == QC_SS_CLO;
 }
 
+/* Allocate if needed buffer <bptr> for stream <qcs>.
+ *
+ * Returns the buffer instance or NULL on allocation failure.
+ */
 struct buffer *qc_get_buf(struct qcs *qcs, struct buffer *bptr)
 {
-	struct buffer *buf = b_alloc(bptr);
-	BUG_ON(!buf);
-	return buf;
+	return b_alloc(bptr);
 }
 
 static struct ncbuf *qc_get_ncbuf(struct qcs *qcs, struct ncbuf *ncbuf)
@@ -1411,7 +1413,7 @@ static void qcs_destroy(struct qcs *qcs)
 /* Transfer as much as possible data on <qcs> from <in> to <out>. This is done
  * in respect with available flow-control at stream and connection level.
  *
- * Returns the total bytes of transferred data.
+ * Returns the total bytes of transferred data or a negative error code.
  */
 static int qcs_xfer_data(struct qcs *qcs, struct buffer *out, struct buffer *in)
 {
@@ -1421,7 +1423,10 @@ static int qcs_xfer_data(struct qcs *qcs, struct buffer *out, struct buffer *in)
 
 	TRACE_ENTER(QMUX_EV_QCS_SEND, qcc->conn, qcs);
 
-	qc_get_buf(qcs, out);
+	if (!qc_get_buf(qcs, out)) {
+		TRACE_ERROR("buffer alloc failure", QMUX_EV_QCS_SEND, qcc->conn, qcs);
+		goto err;
+	}
 
 	/*
 	 * QCS out buffer diagram
@@ -1473,6 +1478,10 @@ static int qcs_xfer_data(struct qcs *qcs, struct buffer *out, struct buffer *in)
 	}
 
 	return total;
+
+ err:
+	TRACE_DEVEL("leaving on error", QMUX_EV_QCS_SEND, qcc->conn, qcs);
+	return -1;
 }
 
 /* Prepare a STREAM frame for <qcs> instance using <out> as payload. The frame
@@ -1838,6 +1847,9 @@ static int _qc_send_qcs(struct qcs *qcs, struct list *frms)
 
 		/* Transfer data from <buf> to <out>. */
 		xfer = qcs_xfer_data(qcs, out, buf);
+		if (xfer < 0)
+			goto err;
+
 		if (xfer > 0) {
 			qcs_notify_send(qcs);
 			qcs->flags &= ~QC_SF_BLK_MROOM;