From ce5c145df5cd77a8a79ab0980c8a5c8010128abc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Wed, 5 Apr 2023 09:44:21 +0200 Subject: [PATCH] BUG/MINOR: quic: Possible crashes in qc_idle_timer_task() This is due to this commit: MINOR: quic: Add trace to debug idle timer task issues where has been added without having been tested at developer level. was dereferenced after having been released by qc_conn_release(). Set qc to NULL value after having been released to forbid its dereferencing. Add a check for qc->idle_timer_task in the traces added by the mentionned commit above to prevent its dereferencing if NULL. Take the opportunity of this patch to modify trace events from QUIC_EV_CONN_SSLALERT to QUIC_EV_CONN_IDLE_TIMER. Must be backported to 2.6 and 2.7. --- src/quic_conn.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/quic_conn.c b/src/quic_conn.c index d858b437f..fb791dd6d 100644 --- a/src/quic_conn.c +++ b/src/quic_conn.c @@ -734,7 +734,7 @@ static void quic_trace(enum trace_level level, uint64_t mask, const struct trace if (tick_isset(qc->idle_expire)) chunk_appendf(&trace_buf, " idle_expire=%ums", TICKS_TO_MS(tick_remain(now_ms, qc->idle_expire))); - if (tick_isset(qc->idle_timer_task->expire)) + if (qc->idle_timer_task && tick_isset(qc->idle_timer_task->expire)) chunk_appendf(&trace_buf, " expire=%ums", TICKS_TO_MS(tick_remain(now_ms, qc->idle_timer_task->expire))); } @@ -5685,7 +5685,7 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state) goto requeue; if (tick_is_expired(qc->ack_expire, now_ms)) { - TRACE_PROTO("ack timer expired", QUIC_EV_CONN_SSLALERT, qc); + TRACE_PROTO("ack timer expired", QUIC_EV_CONN_IDLE_TIMER, qc); qc->ack_expire = TICK_ETERNITY; /* Note that ->idle_expire is always set. */ t->expire = qc->idle_expire; @@ -5704,8 +5704,10 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state) * responsible to call quic_close to release it. */ qc->flags |= QUIC_FL_CONN_EXP_TIMER; - if (qc->mux_state != QC_MUX_READY) + if (qc->mux_state != QC_MUX_READY) { quic_conn_release(qc); + qc = NULL; + } /* TODO if the quic-conn cannot be freed because of the MUX, we may at * least clean some parts of it such as the tasklet. @@ -5713,7 +5715,7 @@ struct task *qc_idle_timer_task(struct task *t, void *ctx, unsigned int state) if (!(qc_flags & QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED)) { qc_flags |= QUIC_FL_CONN_HALF_OPEN_CNT_DECREMENTED; - TRACE_DEVEL("dec half open counter", QUIC_EV_CONN_SSLALERT, qc); + TRACE_DEVEL("dec half open counter", QUIC_EV_CONN_IDLE_TIMER, qc); HA_ATOMIC_DEC(&prx_counters->half_open_conn); }