From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Thu, 23 Jun 2022 19:05:05 +0000 (+0200)
Subject: BUG/MAJOR: quic: Big RX dgrams leak with POST requests
X-Git-Tag: v2.7-dev1~3
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2bed1f166e0f4fd865007a8ad86be7263b019427;p=thirdparty%2Fhaproxy.git

BUG/MAJOR: quic: Big RX dgrams leak with POST requests

This previous commit:
 "BUG/MAJOR: Big RX dgrams leak when fulfilling a buffer"
partially fixed an RX dgram memleak. There is a missing break in the loop which
looks for the first datagram attached to an RX buffer dgrams list which may be
reused (because consumed by the connection thread). So when several dgrams were
consumed by the connection thread and are present in the RX buffer list, some are
leaked because never reused for ever. They are removed for their list.

Furthermore, as commented in this patch, there is always at least one dgram
object attached to an RX dgrams list, excepted the first time we enter this
I/O handler function for this RX buffer. So, there is no need to use a loop
to lookup and reuse the first datagram in an RX buffer dgrams list.

This isssue was reproduced with quiche client with plenty of POST requests
(100000 streams):
 cargo run --bin quiche-client -- https://127.0.0.1:8080/helloworld.html
 --no-verify -n 100000 --method POST --body /var/www/html/helloworld.html
and could be reproduce with GET request.

This bug was reported by Tristan in GH #1749.

Must be backported to 2.6.
---

diff --git a/src/quic_sock.c b/src/quic_sock.c
index 090cec1bb5..f8590f8462 100644
--- a/src/quic_sock.c
+++ b/src/quic_sock.c
@@ -273,7 +273,7 @@ void quic_sock_fd_iocb(int fd)
 	struct sockaddr_storage saddr = {0};
 	size_t max_sz, cspace;
 	socklen_t saddrlen;
-	struct quic_dgram *dgram, *dgramp, *new_dgram;
+	struct quic_dgram *new_dgram;
 	unsigned char *dgram_buf;
 
 	BUG_ON(!l);
@@ -291,15 +291,19 @@ void quic_sock_fd_iocb(int fd)
 
 	buf = &rxbuf->buf;
 
-	/* Try to reuse an existing dgram */
-	list_for_each_entry_safe(dgram, dgramp, &rxbuf->dgrams, list) {
-		if (HA_ATOMIC_LOAD(&dgram->buf))
-			break;
-
-		LIST_DELETE(&dgram->list);
-		b_del(buf, dgram->len);
-		if (!new_dgram)
-			new_dgram = dgram;
+	/* Try to reuse an existing dgram. Note that there is alway at
+	 * least one datagram to pick, except the first time we enter
+	 * this function for this <rxbuf> buffer.
+	 */
+	if (!LIST_ISEMPTY(&rxbuf->dgrams)) {
+		struct quic_dgram *dg =
+			LIST_ELEM(rxbuf->dgrams.n, struct quic_dgram *, list);
+
+		if (!dg->buf) {
+			LIST_DELETE(&dg->list);
+			b_del(buf, dg->len);
+			new_dgram = dg;
+		}
 	}
 
 	params = &l->bind_conf->quic_params;