]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: quic: Possible crashes in qc_idle_timer_task()
authorFrédéric Lécaille <flecaille@haproxy.com>
Wed, 5 Apr 2023 07:44:21 +0000 (09:44 +0200)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Wed, 5 Apr 2023 09:03:20 +0000 (11:03 +0200)
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.

src/quic_conn.c

index d858b437f07feaf8e77d4b1a46b1e09af4e23f51..fb791dd6d2a29e1683d67ab42ab63b73f24ba708 100644 (file)
@@ -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);
        }