mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-08-12 10:06:58 +02:00
Code Issues Projects Releases Wiki Activity

MINOR: quic: Race issue when consuming RX packets buffer

Browse Source 
Add a null byte to the end of the RX buffer to notify the consumer there is no
more data to treat.
Modify quic_rx_packet_pool_purge() which is the function which remove the
RX packet from the buffer.
Also rename this function to quic_rx_pkts_del().
As the RX packets may be accessed by the QUIC connection handler (quic_conn_io_cb())
the function responsible of decrementing their reference counters must not
access other information than these reference counters! It was a very bad idea
to try to purge the RX buffer asap when executing this function.
This commit is contained in:
Frédéric Lécaille 2021-12-02 14:46:19 +01:00 committed by Amaury Denoyelle
parent f9cb3a9b0e
commit d61bc8db59
2 changed files with 20 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

38
include/haproxy/xprt_quic.h
View File

@ -1043,13 +1043,21 @@ static inline int qc_pkt_long(const struct quic_rx_packet *pkt)
* for the connection. * for the connection.
* Always succeeds. * Always succeeds.
*/ */
static inline void quic_rx_packet_pool_purge(struct quic_conn *qc) static inline void quic_rx_pkts_del(struct quic_conn *qc)
{ {
struct quic_rx_packet *pkt, *pktback; struct quic_rx_packet *pkt, *pktback;
list_for_each_entry_safe(pkt, pktback, &qc->rx.pkt_list, qc_rx_pkt_list) { list_for_each_entry_safe(pkt, pktback, &qc->rx.pkt_list, qc_rx_pkt_list) {
if (pkt->data != (unsigned char *)b_head(&qc->rx.buf)) if (pkt->data != (unsigned char *)b_head(&qc->rx.buf)) {
size_t cdata;
cdata = b_contig_data(&qc->rx.buf, 0);
if (cdata && !*b_head(&qc->rx.buf)) {
/* Consume the remaining data */
b_del(&qc->rx.buf, cdata);
}
break; break;
}
if (!HA_ATOMIC_LOAD(&pkt->refcnt)) { if (!HA_ATOMIC_LOAD(&pkt->refcnt)) {
b_del(&qc->rx.buf, pkt->raw_len); b_del(&qc->rx.buf, pkt->raw_len);
@ -1065,30 +1073,14 @@ static inline void quic_rx_packet_refinc(struct quic_rx_packet *pkt)
HA_ATOMIC_ADD(&pkt->refcnt, 1); HA_ATOMIC_ADD(&pkt->refcnt, 1);
} }
/* Decrement the reference counter of <pkt> */ /* Decrement the reference counter of <pkt> while remaining positive */
static inline void quic_rx_packet_refdec(struct quic_rx_packet *pkt) static inline void quic_rx_packet_refdec(struct quic_rx_packet *pkt)
{ {
if (HA_ATOMIC_SUB_FETCH(&pkt->refcnt, 1)) int refcnt;
return;
if (!pkt->qc) { do {
/* It is possible the connection for this packet has not already been refcnt = HA_ATOMIC_LOAD(&pkt->refcnt);
* identified. In such a case, we only need to free this packet. } while (refcnt && !HA_ATOMIC_CAS(&pkt->refcnt, &refcnt, refcnt - 1));
*/
pool_free(pool_head_quic_rx_packet, pkt);
}
else {
struct quic_conn *qc = pkt->qc;
HA_RWLOCK_WRLOCK(QUIC_LOCK, &qc->rx.buf_rwlock);
if (pkt->data == (unsigned char *)b_head(&qc->rx.buf)) {
b_del(&qc->rx.buf, pkt->raw_len);
LIST_DELETE(&pkt->qc_rx_pkt_list);
pool_free(pool_head_quic_rx_packet, pkt);
quic_rx_packet_pool_purge(qc);
}
HA_RWLOCK_WRUNLOCK(QUIC_LOCK, &qc->rx.buf_rwlock);
}
} }
/* Increment the reference counter of <pkt> */ /* Increment the reference counter of <pkt> */

5
src/xprt_quic.c
View File

@ -4121,9 +4121,14 @@ static ssize_t qc_lstnr_pkt_rcv(unsigned char **buf, const unsigned char *end,
} }
HA_RWLOCK_WRLOCK(QUIC_LOCK, &qc->rx.buf_rwlock); HA_RWLOCK_WRLOCK(QUIC_LOCK, &qc->rx.buf_rwlock);
quic_rx_pkts_del(qc);
b_cspace = b_contig_space(&qc->rx.buf); b_cspace = b_contig_space(&qc->rx.buf);
if (b_cspace < pkt->len) { if (b_cspace < pkt->len) {
/* Let us consume the remaining contiguous space. */ /* Let us consume the remaining contiguous space. */
if (b_cspace) {
b_putchr(&qc->rx.buf, 0x00);
b_cspace--;
}
b_add(&qc->rx.buf, b_cspace); b_add(&qc->rx.buf, b_cspace);
if (b_contig_space(&qc->rx.buf) < pkt->len) { if (b_contig_space(&qc->rx.buf) < pkt->len) {
HA_RWLOCK_WRUNLOCK(QUIC_LOCK, &qc->rx.buf_rwlock); HA_RWLOCK_WRUNLOCK(QUIC_LOCK, &qc->rx.buf_rwlock);