From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Thu, 20 Jan 2022 16:43:02 +0000 (+0100)
Subject: MINOR: quic: fix race-condition on xprt tasklet free
X-Git-Tag: v2.6-dev1~102
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=760da3be57301cddc96c7b1e517dbb2e00a5ede1;p=thirdparty%2Fhaproxy.git

MINOR: quic: fix race-condition on xprt tasklet free

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

diff --git a/src/xprt_quic.c b/src/xprt_quic.c
index 7ea5f09405..21cff9cf71 100644
--- a/src/xprt_quic.c
+++ b/src/xprt_quic.c
@@ -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