From 00c346726a2a168fd47eeab3040890e548146943 Mon Sep 17 00:00:00 2001 From: Frederic Lecaille Date: Mon, 25 Nov 2024 11:14:20 +0100 Subject: [PATCH] BUG/MAJOR: quic: fix wrong packet building due to already acked frames If a packet build was asked to probe the peer with frames which have just been acked, the frames build run by qc_build_frms() could be cancelled by qc_stream_frm_is_acked() whose aim is to check that current frames to be built have not been already acknowledged. In this case the packet build run by qc_do_build_pkt() is not interrupted, leading to the build of an empty packet which should be ack-eliciting. This is a bug detected by the BUG_ON() statement in qc_do_build_pk(): BUG_ON(qel->pktns->tx.pto_probe && !(pkt->flags & QUIC_FL_TX_PACKET_ACK_ELICITING)); Thank you to @Tristan971 for having reported this issue in GH #2709 This is an old bug which must be backported as far as 2.6. (cherry picked from commit 96b2641fc8ce58eb1875e7b525c57e58e4b794c3) Signed-off-by: Christopher Faulet --- src/quic_tx.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index d5157c03b..b93d7a1c6 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1863,7 +1863,23 @@ static int qc_do_build_pkt(unsigned char *pos, const unsigned char *end, goto comp_pkt_len; } - if (!ack_frm_len && !qel->pktns->tx.pto_probe) + if (qel->pktns->tx.pto_probe) { + /* If a probing packet was asked and could not be built, + * this is not because there was not enough room, but due to + * its frames which were already acknowledeged. + * See qc_stream_frm_is_acked()) called by qc_build_frms(). + * Note that qc_stream_frm_is_acked() logs a trace in this + * case mentionning some frames were already acknowledged. + * + * That said, the consequence must be the same: cancelling + * the packet build as if there was not enough room in the + * TX buffer. + */ + qel->pktns->tx.pto_probe--; + goto no_room; + } + + if (!ack_frm_len) goto no_room; } }