BUG/MEDIUM: h3: do not access QCS <sd> if not allocated

Since the following commit, allocation of QCS stream-endpoint on FE side
has been delayed. The objective is to allocate it only for QCS attached
to an upper stream object. Stream-endpoint allocation is now performed
on qcs_attach_sc() called during HEADERS parsing.

  commit e6064c561684d9b079e3b5725d38dc3b5c1b5cd5
  OPTIM: mux-quic: delay FE sedesc alloc to stream creation

Also, stream-endpoint is accessed through the QCS instance after HEADERS
or DATA frames parsing, to update the known input payload length. The
above patch triggered regressions as in some code paths, <sd> field is
dereferenced while still being NULL.

This patch fixes this by restricting access to <sd> field after newer
conditions.

First, after HEADERS parsing, known input length is only updated if
h3_req_headers_to_htx() previously returned a success value, which
guarantee that qcs_attach_sc() has been executed.

After DATA parsing, <sd> is only accessed after the frame validity
check. This ensures that HEADERS were already parsed, thus guaranteing
that stream-endpoint is allocated.

This should fix github issue #3211.

This must be backported up to 3.3. This is sufficient, unless above
patch is backported to previous releases, in which case the current one
must be picked with it.

diff --git a/src/h3.c b/src/h3.c
index 9ad5ff430b46e12c2d682ff80c5ec0c58d40c3d7..c0a51d365c344bd9ce06468ee5c5a5affb2e33a2 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1767,7 +1767,14 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        total += hlen;
                        TRACE_PROTO("parsing a new frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
 
-                       if (ftype == H3_FT_DATA) {
+                       if ((ret = h3_check_frame_valid(h3c, qcs, ftype))) {
+                               TRACE_ERROR("received an invalid frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                               qcc_set_error(qcs->qcc, ret, 1);
+                               qcc_report_glitch(qcs->qcc, 1);
+                               goto err;
+                       }
+
+                       if (h3s->type == H3S_T_REQ && ftype == H3_FT_DATA) {
                                h3s->data_len += flen;
 
                                if (h3s->flags & H3_SF_HAVE_CLEN) {
@@ -1778,18 +1785,14 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                                break;
                                }
                                else {
-                                       /* content-length not present, update estimated payload length. */
+                                       /* content-length not present, update estimated payload length.
+                                        * <qcs.sd> is valid here as HEADERS frame were already parsed,
+                                        * guaranteed by h3_check_frame_valid().
+                                        */
                                        qcs->sd->kip = h3s->data_len;
                                }
                        }
 
-                       if ((ret = h3_check_frame_valid(h3c, qcs, ftype))) {
-                               TRACE_ERROR("received an invalid frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
-                               qcc_set_error(qcs->qcc, ret, 1);
-                               qcc_report_glitch(qcs->qcc, 1);
-                               goto err;
-                       }
-
                        if (!b_data(b))
                                break;
                }
@@ -1849,8 +1852,10 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                        }
                                }
 
-                               /* Update estimated payload with content-length value if present. */
-                               if (h3s->flags & H3_SF_HAVE_CLEN)
+                               /* Update estimated payload with content-length value if present.
+                                * <sd> must be allocated on h3_req_headers_to_htx() success.
+                                */
+                               if (ret >= 0 && h3s->flags & H3_SF_HAVE_CLEN)
                                        qcs->sd->kip = h3s->body_len;
                        }
                        else {