From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Wed, 5 Apr 2023 07:44:21 +0000 (+0200)
Subject: BUG/MINOR: quic: Possible crashes in qc_idle_timer_task()
X-Git-Tag: v2.8-dev7~22
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ce5c145df5cd77a8a79ab0980c8a5c8010128abc;p=thirdparty%2Fhaproxy.git

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.
<qc> 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.
---

diff --git a/src/quic_conn.c b/src/quic_conn.c
index d858b437f0..fb791dd6d2 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);
 	}