Skip to content

Commit

Permalink
BUG/MEDIUM: quic: Possible crash for connections to be killed
Browse files Browse the repository at this point in the history
The connections are flagged as "to be killed" asap when the peer has left
(detected by sendto() "Connection refused" errno) by qc_kill_conn(). This
function has to wakeup the idle timer task to release the connection (and the idle
timer  and the idle timer task itself). Then if in the meantime the connection
was flagged as having to process some retransmissions, some packet could lead
to sendto() errors again with a call to qc_kill_conn(), this time with a released
idle timer task.

This bug could be detected by libasan as follows:

.AddressSanitizer:DEADLYSIGNAL
=================================================================
==21018==ERROR: AddressSanitizer: SEGV on unknown address 0x000000000000 (pc 0x    560b5d898717 bp 0x7f9aaac30000 sp 0x7f9aaac2ff80 T3)
==21018==The signal is caused by a READ memory access.
==21018==Hint: address points to the zero page.
.    #0 0x560b5d898717 in _task_wakeup include/haproxy/task.h:209
    joyent#1 0x560b5d8a563c in qc_kill_conn src/quic_conn.c:171
    joyent#2 0x560b5d97f832 in qc_send_ppkts src/quic_tx.c:636
    haproxy#3 0x560b5d981b53 in qc_send_app_pkts src/quic_tx.c:876
    haproxy#4 0x560b5d987122 in qc_send_app_probing src/quic_tx.c:910
    haproxy#5 0x560b5d987122 in qc_dgrams_retransmit src/quic_tx.c:1397
    haproxy#6 0x560b5d8ab250 in quic_conn_app_io_cb src/quic_conn.c:712
    haproxy#7 0x560b5de41593 in run_tasks_from_lists src/task.c:596
    haproxy#8 0x560b5de4333c in process_runnable_tasks src/task.c:876
    haproxy#9 0x560b5dd6f2b0 in run_poll_loop src/haproxy.c:2966
    haproxy#10 0x560b5dd700fb in run_thread_poll_loop src/haproxy.c:3165
    haproxy#11 0x7f9ab9188ea6 in start_thread nptl/pthread_create.c:477
    haproxy#12 0x7f9ab90a8a2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfba2e)

AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV include/haproxy/task.h:209 in _task_wakeup
Thread T3 created by T0 here:
    #0 0x7f9ab97ac2a2 in __interceptor_pthread_create ../../../../src/libsaniti    zer/asan/asan_interceptors.cpp:214
    joyent#1 0x560b5df4f3ef in setup_extra_threads src/thread.c:252                  o
    joyent#2 0x560b5dd730c7 in main src/haproxy.c:3856
    haproxy#3 0x7f9ab8fd0d09 in __libc_start_main ../csu/libc-start.c:308             i

==21018==ABORTING
AddressSanitizer:DEADLYSIGNAL
Aborted (core dumped)

To fix, simply reset the connection flag QUIC_FL_CONN_RETRANS_NEEDED to cancel
the retransmission when qc_kill_conn is called.

Note that this new bug arrived with this fix which is correct and flagged as to be
backported as far as 2.6.
      BUG/MINOR: quic: idle timer task requeued in the past

Must be backported as far as 2.6.

(cherry picked from commit 756b3c5)
Signed-off-by: Christopher Faulet <[email protected]>
  • Loading branch information
haproxyFred authored and capflam committed Dec 5, 2023
1 parent 3ecb341 commit f4bec87
Showing 1 changed file with 1 addition and 0 deletions.
1 change: 1 addition & 0 deletions src/quic_conn.c
Original file line number Diff line number Diff line change
Expand Up @@ -842,6 +842,7 @@ void qc_kill_conn(struct quic_conn *qc)
TRACE_ENTER(QUIC_EV_CONN_KILL, qc);
TRACE_PROTO("killing the connection", QUIC_EV_CONN_KILL, qc);
qc->flags |= QUIC_FL_CONN_TO_KILL;
qc->flags &= ~QUIC_FL_CONN_RETRANS_NEEDED;
task_wakeup(qc->idle_timer_task, TASK_WOKEN_OTHER);

qc_notify_err(qc);
Expand Down

0 comments on commit f4bec87

Please sign in to comment.