From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Mon, 16 May 2022 16:13:56 +0000 (+0200)
Subject: BUG/MEDIUM: quic: fix Rx buffering
X-Git-Tag: v2.6-dev11~65
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=80d0572a3151f3cc9799d924aab5b22542d35f31;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: quic: fix Rx buffering

The quic-conn manages a buffer to store received QUIC packets. When the
buffer wraps, the gap is filled until the end with junk and packets can
be inserted at the start of the buffer.

On the other end, deletion is implemented via quic_rx_pkts_del().
Packets are removed one by one if their refcount is nul. If junk is
found, the buffer is emptied until its wrap.

This seems to work in most cases but a bug was found in a particular
case : on insertion if buffer gap is not at the end of the buffer. In
this case, the gap was filled, which is useless as now the buffer is
full and the packet cannot be inserted. Worst, on deletion, when junk is
removed there is a risk to removed new packets. This can happens in the
following case :
1. buffer contig space is too small, junk is inserted in the middle of
   it
2. on quic_rx_pkts_del() invocation, a packet is removed, but not the
   next one because its refcount is still positive. When a new packet is
   received, it will be stored after the junk.
3. on next quic_rx_pkts_del(), when junk is removed, all contig data is
   cleared, with newer packets data too.

This will cause a transfer between a client and haproxy to be stalled.
This can be reproduced with big enough POST requests. I triggered it
with ngtcp2 and 10M of posted data.

Hopefully, the solution of this bug is simple. If contig space is not
big enough to store a packet, but the space is not at the end of the
buffer, no junk is inserted and the packet is dropped as we cannot
buffered it. This ensures that junk is only present at the end of the
buffer and when removed no packets data is purged with it.
---

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index bc290faead..ed6321865e 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -5315,6 +5315,12 @@ static void qc_lstnr_pkt_rcv(unsigned char *buf, const unsigned char *end,
 	quic_rx_pkts_del(qc);
 	b_cspace = b_contig_space(&qc->rx.buf);
 	if (b_cspace < pkt->len) {
+		/* Do not consume buf if space not at the end. */
+		if (b_tail(&qc->rx.buf) + b_cspace < b_wrap(&qc->rx.buf)) {
+			TRACE_PROTO("Packet dropped", QUIC_EV_CONN_LPKT, qc);
+			goto err;
+		}
+
 		/* Let us consume the remaining contiguous space. */
 		if (b_cspace) {
 			b_putchr(&qc->rx.buf, 0x00);