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

BUG/MAJOR: quic: Crash after discarding packet number spaces

Browse Source 
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.

(cherry picked from commit 74b5f7b31b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
This commit is contained in:
Frédéric Lécaille 2022-11-20 18:35:35 +01:00 committed by Christopher Faulet
parent 05e719b7bd
commit 38c47fb838
2 changed files with 19 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
include/haproxy/quic_conn.h
View File

@ -458,6 +458,19 @@ static inline int64_t quic_pktns_get_largest_acked_pn(struct quic_pktns *pktns)
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)
{
@ -469,6 +482,10 @@ static inline void quic_tx_packet_refdec(struct quic_tx_packet *pkt)
{
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);
}
}

7
src/quic_conn.c
View File

@ -1759,10 +1759,7 @@ static inline struct eb64_node *qc_ackrng_pkts(struct quic_conn *qc,
/* 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 @@ static int qc_prep_pkts(struct quic_conn *qc, struct buffer *buf,
/* 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;