From: Willy Tarreau <w@1wt.eu>
Date: Wed, 7 May 2025 16:43:57 +0000 (+0200)
Subject: BUG/MEDIUM: stick-tables: close a tiny race in __stksess_kill()
X-Git-Tag: v3.2-dev15~7
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0cee7b5b8d32c307f1feaeb72491597443f5a35b;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: stick-tables: close a tiny race in __stksess_kill()

It might be possible not to see the element in the tree, then not to
see it in the update list, thus not to take the lock before deleting.
But an element in the list could have moved to the tree during the
check, and be removed later without the updt_lock.

Let's delete prior to checking the presence in the tree to avoid
this situation. No backport is needed since this arrived in -dev13
with the update list.
---

diff --git a/src/stick_table.c b/src/stick_table.c
index bf6ae3989..26747eac3 100644
--- a/src/stick_table.c
+++ b/src/stick_table.c
@@ -144,13 +144,20 @@ int __stksess_kill(struct stktable *t, struct stksess *ts)
 	if (HA_ATOMIC_LOAD(&ts->ref_cnt))
 		return 0;
 
-	if (ts->upd.node.leaf_p || !MT_LIST_ISEMPTY(&ts->pend_updts)) {
+	/* make sure we're no longer in the updates list */
+	MT_LIST_DELETE(&ts->pend_updts);
+
+	/* ... and that nobody added us in between */
+	if (HA_ATOMIC_LOAD(&ts->ref_cnt))
+		return 0;
+
+	/* ... and that we didn't leave the update list for the tree */
+	if (ts->upd.node.leaf_p) {
 		updt_locked = 1;
 		HA_RWLOCK_WRLOCK(STK_TABLE_UPDT_LOCK, &t->updt_lock);
 		if (HA_ATOMIC_LOAD(&ts->ref_cnt))
 			goto out_unlock;
 	}
-	MT_LIST_DELETE(&ts->pend_updts);
 	eb32_delete(&ts->exp);
 	eb32_delete(&ts->upd);
 	ebmb_delete(&ts->key);