From e9a974a37a58eb3a23316bd03e0b95e00d951cb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Sun, 13 Mar 2022 19:19:12 +0100 Subject: [PATCH] BUG/MAJOR: quic: Possible crash with full congestion control window This commit reverts this one: "d5066dd9d BUG/MEDIUM: quic: qc_prep_app_pkts() retries on qc_build_pkt() failures" After having filled the congestion control window, qc_build_pkt() always fails. Then depending on the relative position of the writer and reader indexes for the TX buffer, this could lead this function to try to reuse the buffer even if not full. In such case, we do not always mark the end of the data in this TX buffer. This is something the reader cannot understand: it reads a false datagram length, then a wrong packet address from the TX buffer, leading to an invalid pointer dereferencing. --- src/xprt_quic.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/xprt_quic.c b/src/xprt_quic.c index bfe888f3b..34fc08423 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -2528,7 +2528,11 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct qring *qr, case -2: goto err; case -1: - goto stop_build; + /* As we provide qc_build_pkt() with an enough big buffer to fulfill an + * MTU, we are here because of the congestion control window. There is + * no need to try to reuse this buffer. + */ + goto out; default: break; } @@ -2542,7 +2546,6 @@ static int qc_prep_app_pkts(struct quic_conn *qc, struct qring *qr, qc_set_dg(cbuf, pkt->len, pkt); } - stop_build: /* Reset writer index if in front of index */ if (end_buf - pos < (int)qc->path->mtu + dg_headlen) { int rd = HA_ATOMIC_LOAD(&cbuf->rd); @@ -5204,6 +5207,10 @@ static inline void free_quic_tx_packet(struct quic_tx_packet *pkt) * * Return -2 if the packet could not be allocated or encrypted for any reason, * -1 if there was not enough room to build a packet. + * XXX NOTE XXX + * If you provide provide qc_build_pkt() with a big enough buffer to build a packet as big as + * possible (to fill an MTU), the unique reason why this function may fail is the congestion + * control window limitation. */ static struct quic_tx_packet *qc_build_pkt(unsigned char **pos, const unsigned char *buf_end,