]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: quic: fix race-condition on xprt tasklet free
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Thu, 20 Jan 2022 16:43:02 +0000 (17:43 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 21 Jan 2022 14:19:31 +0000 (15:19 +0100)
Remove the unsafe call to tasklet_free in quic_close. At this stage the
tasklet may already be scheduled by an other threads even after if the
quic_conn refcount is now null. It will probably cause a crash on the
next tasklet processing.

Use tasklet_kill instead to ensure that the tasklet is freed in a
thread-safe way. Note that quic_conn_io_cb is not protected by the
refcount so only the quic_conn pinned thread must kill the tasklet.

src/xprt_quic.c

index 7ea5f09405fe59e9bd626604013f124fcd98ec4f..21cff9cf71dd898a5245bdc8c68d7894359c0bac 100644 (file)
@@ -3403,13 +3403,15 @@ static void quic_conn_drop(struct quic_conn *qc)
 /* Release the quic_conn <qc>. It will decrement its refcount so that the
  * connection will be freed once all threads have finished to work with it. The
  * connection is removed from the CIDs tree and thus cannot be found by other
- * threads after it.
+ * threads after it. The connection tasklet is killed.
  *
  * Do not use <qc> after it as it may be freed. This function must only be
  * called by the thread responsible of the quic_conn tasklet.
  */
 static void quic_conn_release(struct quic_conn *qc)
 {
+       struct ssl_sock_ctx *conn_ctx;
+
        /* remove the connection from receiver cids trees */
        HA_RWLOCK_WRLOCK(QUIC_LOCK, &qc->li->rx.cids_lock);
        ebmb_delete(&qc->odcid_node);
@@ -3417,6 +3419,13 @@ static void quic_conn_release(struct quic_conn *qc)
        free_quic_conn_cids(qc);
        HA_RWLOCK_WRUNLOCK(QUIC_LOCK, &qc->li->rx.cids_lock);
 
+       /* Kill the tasklet. Do not use tasklet_free as this is not thread safe
+        * as other threads may call tasklet_wakeup after this.
+        */
+       conn_ctx = HA_ATOMIC_LOAD(&qc->xprt_ctx);
+       if (conn_ctx)
+               tasklet_kill(conn_ctx->wait_event.tasklet);
+
        quic_conn_drop(qc);
 }
 
@@ -3432,11 +3441,6 @@ void quic_close(struct connection *conn, void *xprt_ctx)
                qc->timer_task = NULL;
        }
 
-       if (conn_ctx->wait_event.tasklet) {
-               tasklet_free(conn_ctx->wait_event.tasklet);
-               conn_ctx->wait_event.tasklet = NULL;
-       }
-
        TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
 
        /* TODO for now release the quic_conn on notification by the upper