From 89d685f396b752d7af8a09e0a19a8a469a2cf2d4 Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Thu, 12 Oct 2023 18:15:01 +0200 Subject: [PATCH] BUG/MEDIUM: quic-conn: free unsent frames on retransmit to prevent crash Since the following patch : commit 33c49cec987c1dcd42d216c6d075fb8260058b16 MINOR: quic: Make qc_dgrams_retransmit() return a status. retransmission process is interrupted as soon as a fatal send error has been encounted. However, this may leave frames in local list. This cause several issues : a memory leak and a potential crash. The crash happens because leaked frames are duplicated of an origin frame via qc_dup_pkt_frms(). If an ACK arrives later for the origin frame, all duplicated frames are also freed. During qc_frm_free(), LIST_DEL_INIT() operation is invalid as it still references the local list used inside qc_dgrams_retransmit(). This bug was reproduced using the following injection from another machine : $ h2load --npn-list h3 -t 8 -c 10000 -m 1 -n 2000000000 \ https://:/?s=4m Haproxy was compiled using ASAN. The crash resulted in the following trace : ==332748==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7fff82bf9d78 at pc 0x556facd3b95a bp 0x7fff82bf8b20 sp 0x7fff82bf8b10 WRITE of size 8 at 0x7fff82bf9d78 thread T0 #0 0x556facd3b959 in qc_frm_free include/haproxy/quic_frame.h:273 #1 0x556facd59501 in qc_release_frm src/quic_conn.c:1724 #2 0x556facd5a07f in quic_stream_try_to_consume src/quic_conn.c:1803 #3 0x556facd5abe9 in qc_treat_acked_tx_frm src/quic_conn.c:1866 #4 0x556facd5b3d8 in qc_ackrng_pkts src/quic_conn.c:1928 #5 0x556facd60187 in qc_parse_ack_frm src/quic_conn.c:2354 #6 0x556facd693a1 in qc_parse_pkt_frms src/quic_conn.c:3203 #7 0x556facd7531a in qc_treat_rx_pkts src/quic_conn.c:4606 #8 0x556facd7a528 in quic_conn_app_io_cb src/quic_conn.c:5059 #9 0x556fad3284be in run_tasks_from_lists src/task.c:596 #10 0x556fad32a3fa in process_runnable_tasks src/task.c:876 #11 0x556fad24a676 in run_poll_loop src/haproxy.c:2968 #12 0x556fad24b510 in run_thread_poll_loop src/haproxy.c:3167 #13 0x556fad24e7ff in main src/haproxy.c:3857 #14 0x7fae30ddd0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x240b2) #15 0x556facc9375d in _start (/opt/haproxy-quic-2.8/haproxy+0x1ea75d) Address 0x7fff82bf9d78 is located in stack of thread T0 at offset 40 in frame #0 0x556facd74ede in qc_treat_rx_pkts src/quic_conn.c:4580 This must be backported up to 2.7. --- src/quic_tx.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/quic_tx.c b/src/quic_tx.c index 53004acbf..9f87aac67 100644 --- a/src/quic_tx.c +++ b/src/quic_tx.c @@ -1378,6 +1378,7 @@ int qc_dgrams_retransmit(struct quic_conn *qc) if (!LIST_ISEMPTY(&frms1)) { apktns->tx.pto_probe = 1; if (!qc_send_app_probing(qc, &frms1)) { + qc_free_frm_list(qc, &frms1); qc_free_frm_list(qc, &frms2); goto leave; } @@ -1387,8 +1388,10 @@ int qc_dgrams_retransmit(struct quic_conn *qc) } if (!LIST_ISEMPTY(&frms2)) { apktns->tx.pto_probe = 1; - if (!qc_send_app_probing(qc, &frms2)) + if (!qc_send_app_probing(qc, &frms2)) { + qc_free_frm_list(qc, &frms2); goto leave; + } /* Put back unsent frames into their packet number spaces */ LIST_SPLICE(&apktns->tx.frms, &frms2); }