gamzinav/haproxy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

BUG/MINOR: quic: Leak in qc_release_lost_pkts() for non in flight TX packets

Browse Source 
Packets with null "in flight" lengths are kept as the others packets as sent
but not already acknowledeged in the by packet number space trees.
But qc_release_lost_pkts() relied on this in fligh length to release the
memory allocated for this packets. We must release the memory allocated for
all the lost packets regardless of their in fligh lengths.

Modify this function to do nothing if the list of lost packets passed
as argument is empty. Stop using <lost_bytes> variable to decide if some packets
memory must be released or not.
Modify the callers to stop checking if this list is empty.

Should helping in fixing memory leak as reported by Tristan in GH #1801.

Must be backported to 2.6.

(cherry picked from commit 277c4629e7896276f3a54b939471bde59d31fff6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Frédéric Lécaille 2022-08-24 17:06:04 +02:00 committed by Christopher Faulet
parent df135f08f1
commit 51cf730615
1 changed files with 8 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/xprt_quic.c
View File

@ -1867,14 +1867,14 @@ static inline void qc_release_lost_pkts(struct quic_conn *qc,
uint64_t now_us)
{
struct quic_tx_packet *pkt, *tmp, *oldest_lost, *newest_lost;
uint64_t lost_bytes;
lost_bytes = 0;
if (LIST_ISEMPTY(pkts))
return;
oldest_lost = newest_lost = NULL;
list_for_each_entry_safe(pkt, tmp, pkts, list) {
struct list tmp = LIST_HEAD_INIT(tmp);
lost_bytes += pkt->in_flight_len;
pkt->pktns->tx.in_flight -= pkt->in_flight_len;
qc->path->prep_in_flight -= pkt->in_flight_len;
qc->path->in_flight -= pkt->in_flight_len;
@ -1916,11 +1916,9 @@ static inline void qc_release_lost_pkts(struct quic_conn *qc,
qc->path->cc.algo->slow_start(&qc->path->cc);
}
if (lost_bytes) {
quic_tx_packet_refdec(oldest_lost);
if (newest_lost != oldest_lost)
quic_tx_packet_refdec(newest_lost);
}
quic_tx_packet_refdec(oldest_lost);
if (newest_lost != oldest_lost)
quic_tx_packet_refdec(newest_lost);
}
/* Parse ACK frame into <frm> from a buffer at <buf> address with <end> being at
@ -2020,8 +2018,7 @@ static inline int qc_parse_ack_frm(struct quic_conn *qc,
if (!LIST_ISEMPTY(&newly_acked_pkts)) {
if (!eb_is_empty(&qel->pktns->tx.pkts)) {
qc_packet_loss_lookup(qel->pktns, qc, &lost_pkts);
if (!LIST_ISEMPTY(&lost_pkts))
qc_release_lost_pkts(qc, qel->pktns, &lost_pkts, now_ms);
qc_release_lost_pkts(qc, qel->pktns, &lost_pkts, now_ms);
}
qc_treat_newly_acked_pkts(qc, &newly_acked_pkts);
if (quic_peer_validated_addr(qc))
@ -4220,8 +4217,7 @@ static struct task *process_timer(struct task *task, void *ctx, unsigned int sta
struct list lost_pkts = LIST_HEAD_INIT(lost_pkts);
qc_packet_loss_lookup(pktns, qc, &lost_pkts);
if (!LIST_ISEMPTY(&lost_pkts))
qc_release_lost_pkts(qc, pktns, &lost_pkts, now_ms);
qc_release_lost_pkts(qc, pktns, &lost_pkts, now_ms);
qc_set_timer(qc);
goto out;
}