BUG/MAJOR: quic: Crash after discarding packet number spaces
This previous patch was not sufficient to prevent haproxy from
crashing when some Handshake packets had to be inspected before being possibly
retransmitted:
"BUG/MAJOR: quic: Crash upon retransmission of dgrams with several packets"
This patch introduced another issue: access to packets which have been
released because still attached to others (in the same datagram). This was
the case for instance when discarding the Initial packet number space before
inspecting an Handshake packet in the same datagram through its ->prev or
member in our case.
This patch implements quic_tx_packet_dgram_detach() which detaches a packet
from the adjacent ones in the same datagram to be called when ackwowledging
a packet (as done in the previous commit) and when releasing its memory. This
was, we are sure the released packets will not be accessed during retransmissions.
Thank you to @gabrieltz for having reported this issue in GH #1903.
Must be backported to 2.6.
diff --git a/include/haproxy/quic_conn.h b/include/haproxy/quic_conn.h
index a2292d0..6b2b70b 100644
--- a/include/haproxy/quic_conn.h
+++ b/include/haproxy/quic_conn.h
@@ -487,6 +487,19 @@
return eb64_entry(ar, struct quic_arng_node, first)->last;
}
+/* The TX packets sent in the same datagram are linked to each others in
+ * the order they are built. This function detach a packet from its successor
+ * and predecessor in the same datagram.
+ */
+static inline void quic_tx_packet_dgram_detach(struct quic_tx_packet *pkt)
+{
+ if (pkt->prev)
+ pkt->prev->next = pkt->next;
+ if (pkt->next)
+ pkt->next->prev = pkt->prev;
+}
+
+
/* Increment the reference counter of <pkt> */
static inline void quic_tx_packet_refinc(struct quic_tx_packet *pkt)
{
@@ -498,6 +511,10 @@
{
if (!HA_ATOMIC_SUB_FETCH(&pkt->refcnt, 1)) {
BUG_ON(!LIST_ISEMPTY(&pkt->frms));
+ /* If there are others packet in the same datagram <pkt> is attached to,
+ * detach the previous one and the next one from <pkt>.
+ */
+ quic_tx_packet_dgram_detach(pkt);
pool_free(pool_head_quic_tx_packet, pkt);
}
}
diff --git a/src/quic_conn.c b/src/quic_conn.c
index 1d67248..dd74894 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -1759,10 +1759,7 @@
/* If there are others packet in the same datagram <pkt> is attached to,
* detach the previous one and the next one from <pkt>.
*/
- if (pkt->prev)
- pkt->prev->next = pkt->next;
- if (pkt->next)
- pkt->next->prev = pkt->prev;
+ quic_tx_packet_dgram_detach(pkt);
node = eb64_prev(node);
eb64_delete(&pkt->pn_node);
}
@@ -3237,7 +3234,7 @@
/* keep trace of the first packet in the datagram */
if (!first_pkt)
first_pkt = cur_pkt;
- /* Attach the current one to the previous one */
+ /* Attach the current one to the previous one and vice versa */
if (prv_pkt) {
prv_pkt->next = cur_pkt;
cur_pkt->prev = prv_pkt;