From bbf86be9969e070cb57021d14c8fa638199db73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Mon, 20 Feb 2023 09:28:58 +0100 Subject: [PATCH] BUG/MEDIUM: quic: Missing TX buffer draining from qc_send_ppkts() If the TX buffer (->tx.buf) attached to the connection is not drained, there are chances that this will be detected by qc_txb_release() which triggers a BUG_ON_HOT() when this is the case as follows [00|quic|2|c_conn.c:3477] UDP port unreachable : qc@0x5584f18d6d50 pto_count=0 cwnd=6816 ppif=1046 pif=1046 [00|quic|5|ic_conn.c:749] qc_kill_conn(): entering : qc@0x5584f18d6d50 [00|quic|5|ic_conn.c:752] qc_kill_conn(): leaving : qc@0x5584f18d6d50 [00|quic|5|c_conn.c:3532] qc_send_ppkts(): leaving : qc@0x5584f18d6d50 pto_count=0 cwnd=6816 ppif=1046 pif=1046 FATAL: bug condition "buf && b_data(buf)" matched at src/quic_conn.c:3098 Consume the remaining data in the TX buffer calling b_del(). This bug arrived with this commit: a2c62c314 MINOR: quic: Kill the connections on ICMP (port unreachable) packet receipt Takes also the opportunity of this patch to modify the comments for qc_send_ppkts() which should have arrived with a2c62c314 commit. Must be backported to 2.7 where this latter commit is supposed to be backported. --- src/quic_conn.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/quic_conn.c b/src/quic_conn.c index f40ae25e6..3de0d9005 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -3112,6 +3112,8 @@ static void qc_txb_release(struct quic_conn *qc) /* For the moment sending function is responsible to purge the buffer * entirely. It may change in the future but this requires to be able * to reuse old data. + * For the momemt we do not care to leave data in the buffer for + * a connection which is supposed to be killed asap. */ BUG_ON_HOT(buf && b_data(buf)); @@ -3444,9 +3446,11 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf, /* Send datagrams stored in . * - * This function always returns 1 for success. Even if sendto() syscall failed, - * buffer is drained and packets are considered as emitted. QUIC loss detection - * mechanism is used as a back door way to retry sending. + * This function returns 1 for success. Even if sendto() syscall failed, + * buffer is drained and packets are considered as emitted and this function returns 1 + * There is a unique exception when sendto() fails with ECONNREFUSED as errno, + * this function returns 0. + * QUIC loss detection mechanism is used as a back door way to retry sending. */ int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx) { @@ -3494,6 +3498,7 @@ int qc_send_ppkts(struct buffer *buf, struct ssl_sock_ctx *ctx) /* Let's kill this connection asap. */ TRACE_PROTO("UDP port unreachable", QUIC_EV_CONN_SPPKTS, qc); qc_kill_conn(qc); + b_del(buf, buf->data); goto leave; }