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 d41a20c21df5398f0453fd6dd854d29fb6d9c89b..99c2d76474ec9181bdd9743fb49b7febf2ab4e15 100644 (file)
--- 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 01698f05d0b7482087241785c2b25bc37490d3a2..294a0d7b8294bc97edb843beca6dda16060af09e 100644 (file)
--- 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;