BUG/MINOR: quic: Safer QUIC frame builders

Do not rely on the fact the callers of qc_build_frm() handle their
buffer passed to function the correct way (without leaving garbage).
Make qc_build_frm() update the buffer passed as argument only if
the frame it builds is well formed.

As far as I sse, there is no such callers which does not handle
carefully such buffers.

Must be backported to 2.6.

diff --git a/src/quic_frame.c b/src/quic_frame.c
index f590be2922746f72101e33e8668d8adf51a45555..58e9ebcdcf567fad23e0d76449bcb7134d004ca0 100644 (file)
--- a/src/quic_frame.c
+++ b/src/quic_frame.c
@@ -1122,6 +1122,8 @@ int qc_parse_frm(struct quic_frame *frm, struct quic_rx_packet *pkt,
 
 /* Encode <frm> QUIC frame into <buf> buffer.
  * Returns 1 if succeeded (enough room in <buf> to encode the frame), 0 if not.
+ * The buffer is updated to point to one byte past the end of the built frame
+ * only if succeeded.
  */
 int qc_build_frm(unsigned char **buf, const unsigned char *end,
                  struct quic_frame *frm, struct quic_tx_packet *pkt,
@@ -1129,6 +1131,7 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
 {
        int ret = 0;
        const struct quic_frame_builder *builder;
+       unsigned char *pos = *buf;
 
        TRACE_ENTER(QUIC_EV_CONN_BFRM, qc);
        builder = &quic_frame_builders[frm->type];
@@ -1138,19 +1141,20 @@ int qc_build_frm(unsigned char **buf, const unsigned char *end,
                BUG_ON(!(builder->mask & (1U << pkt->type)));
        }
 
-       if (end <= *buf) {
+       if (end <= pos) {
                TRACE_DEVEL("not enough room", QUIC_EV_CONN_BFRM, qc, frm);
                goto leave;
        }
 
        TRACE_PROTO("frame", QUIC_EV_CONN_BFRM, qc, frm);
-       *(*buf)++ = frm->type;
-       if (!quic_frame_builders[frm->type].func(buf, end, frm, qc)) {
+       *pos++ = frm->type;
+       if (!quic_frame_builders[frm->type].func(&pos, end, frm, qc)) {
                TRACE_DEVEL("frame building error", QUIC_EV_CONN_BFRM, qc, frm);
                goto leave;
        }
 
        pkt->flags |= builder->flags;
+       *buf = pos;
 
        ret = 1;
  leave:

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 4f2a2ea05043ab549272f3e9e8f02ba471f5bcf9..0e9e46e7c7f56cb343bfa40a6aba430ae1c2e26c 100644 (file)
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -6832,9 +6832,7 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
        if (!LIST_ISEMPTY(&frm_list)) {
                struct quic_frame *tmp_cf;
                list_for_each_entry_safe(cf, tmp_cf, &frm_list, list) {
-                       unsigned char *spos = pos;
-
-                       if (!qc_build_frm(&spos, end, cf, pkt, qc)) {
+                       if (!qc_build_frm(&pos, end, cf, pkt, qc)) {
                                ssize_t room = end - pos;
                                TRACE_DEVEL("Not enough room", QUIC_EV_CONN_TXPKT,
                                            qc, NULL, NULL, &room);
@@ -6848,7 +6846,6 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end,
                                break;
                        }
 
-                       pos = spos;
                        quic_tx_packet_refinc(pkt);
                        cf->pkt = pkt;
                }