From 2bed1f166e0f4fd865007a8ad86be7263b019427 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Thu, 23 Jun 2022 21:05:05 +0200 Subject: [PATCH] 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. --- src/quic_sock.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/src/quic_sock.c b/src/quic_sock.c index 090cec1bb..f8590f846 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; + /* 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 buffer. + */ + if (!LIST_ISEMPTY(&rxbuf->dgrams)) { + struct quic_dgram *dg = + LIST_ELEM(rxbuf->dgrams.n, struct quic_dgram *, list); - LIST_DELETE(&dgram->list); - b_del(buf, dgram->len); - if (!new_dgram) - new_dgram = dgram; + if (!dg->buf) { + LIST_DELETE(&dg->list); + b_del(buf, dg->len); + new_dgram = dg; + } } params = &l->bind_conf->quic_params;