From ea4a5cbbdfa71cd287d453dffbdf643846754bba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Sat, 20 Aug 2022 18:59:36 +0200 Subject: [PATCH] BUG/MINOR: mux-quic: Fix memleak on QUIC stream buffer for unacknowledged data Some clients send CONNECTION_CLOSE frame without acknowledging the STREAM data haproxy has sent. In this case, when closing the connection if there were remaining data in QUIC stream buffers, they were not released. Add a boolean option to qc_stream_desc_free() to force the stream buffer memory releasing upon closing connection. Thank you to Tristan for having reported such a memory leak issue in GH #1801. Must be backported to 2.6. --- include/haproxy/quic_stream.h | 2 +- src/quic_stream.c | 12 ++++++------ src/xprt_quic.c | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/include/haproxy/quic_stream.h b/include/haproxy/quic_stream.h index 1107e86f2..cb30b1ae4 100644 --- a/include/haproxy/quic_stream.h +++ b/include/haproxy/quic_stream.h @@ -12,7 +12,7 @@ struct qc_stream_desc *qc_stream_desc_new(uint64_t id, enum qcs_type, void *ctx, struct quic_conn *qc); void qc_stream_desc_release(struct qc_stream_desc *stream); int qc_stream_desc_ack(struct qc_stream_desc **stream, size_t offset, size_t len); -void qc_stream_desc_free(struct qc_stream_desc *stream); +void qc_stream_desc_free(struct qc_stream_desc *stream, int closing); struct buffer *qc_stream_buf_get(struct qc_stream_desc *stream); struct buffer *qc_stream_buf_alloc(struct qc_stream_desc *stream, diff --git a/src/quic_stream.c b/src/quic_stream.c index 205ee76aa..abd977c5b 100644 --- a/src/quic_stream.c +++ b/src/quic_stream.c @@ -63,7 +63,7 @@ void qc_stream_desc_release(struct qc_stream_desc *stream) if (LIST_ISEMPTY(&stream->buf_list)) { /* if no buffer left we can free the stream. */ - qc_stream_desc_free(stream); + qc_stream_desc_free(stream, 0); } else { /* A released stream does not use . */ @@ -131,7 +131,7 @@ int qc_stream_desc_ack(struct qc_stream_desc **stream, size_t offset, size_t len /* Free stream instance if already released and no buffers left. */ if (s->release && LIST_ISEMPTY(&s->buf_list)) { - qc_stream_desc_free(s); + qc_stream_desc_free(s, 0); *stream = NULL; } @@ -139,10 +139,10 @@ int qc_stream_desc_ack(struct qc_stream_desc **stream, size_t offset, size_t len } /* Free the stream descriptor content. This function should be used - * when all its data have been acknowledged or on full connection closing. It - * must only be called after the stream is released. + * when all its data have been acknowledged or on full connection closing if + * boolean is set to 1. It must only be called after the stream is released. */ -void qc_stream_desc_free(struct qc_stream_desc *stream) +void qc_stream_desc_free(struct qc_stream_desc *stream, int closing) { struct qc_stream_buf *buf, *buf_back; struct quic_conn *qc = stream->qc; @@ -154,7 +154,7 @@ void qc_stream_desc_free(struct qc_stream_desc *stream) /* free remaining stream buffers */ list_for_each_entry_safe(buf, buf_back, &stream->buf_list, list) { - if (!(b_data(&buf->buf))) { + if (!(b_data(&buf->buf)) || closing) { b_free(&buf->buf); LIST_DELETE(&buf->list); pool_free(pool_head_quic_stream_buf, buf); diff --git a/src/xprt_quic.c b/src/xprt_quic.c index e1f731da7..f6497f102 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -4457,7 +4457,7 @@ static void quic_conn_release(struct quic_conn *qc) * qc_stream_desc_free will liberate the stream instance. */ BUG_ON(!stream->release); - qc_stream_desc_free(stream); + qc_stream_desc_free(stream, 1); } /* Purge Rx packet list. */