From dfd938bad6d9b7af8ca41d7044ae6e6b787abcbc Mon Sep 17 00:00:00 2001 From: Christopher Faulet Date: Tue, 21 May 2024 14:21:58 +0200 Subject: [PATCH] BUG/MEDIUM: stick-tables: Fix race with peers when trashing oldest entries It is the same that the one fixed in process_table_expire() (21447b1dd4 ["BUG/MAJOR: stick-tables: fix race with peers in entry expiration"]). In stktable_trash_oldest(), when the update lock is acquired, we must take care to check again the ref_cnt because some peers may increment it (See commit above for details). This patch fixes a crash mentionned in 2552#issuecomment-2110532706. It must be backported to 2.9. --- src/stick_table.c | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/stick_table.c b/src/stick_table.c index a1cc03c0e..0d353e2e3 100644 --- a/src/stick_table.c +++ b/src/stick_table.c @@ -264,6 +264,7 @@ int stktable_trash_oldest(struct stktable *t, int to_batch) int max_per_shard = (to_batch + CONFIG_HAP_TBL_BUCKETS - 1) / CONFIG_HAP_TBL_BUCKETS; int done_per_shard; int batched = 0; + int updt_locked; int looped; int shard; @@ -272,6 +273,7 @@ int stktable_trash_oldest(struct stktable *t, int to_batch) while (batched < to_batch) { done_per_shard = 0; looped = 0; + updt_locked = 0; HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock); @@ -328,18 +330,35 @@ int stktable_trash_oldest(struct stktable *t, int to_batch) continue; } + /* if the entry is in the update list, we must be extremely careful + * because peers can see it at any moment and start to use it. Peers + * will take the table's updt_lock for reading when doing that, and + * with that lock held, will grab a ref_cnt before releasing the + * lock. So we must take this lock as well and check the ref_cnt. + */ + if (ts->upd.node.leaf_p) { + if (!updt_locked) { + updt_locked = 1; + HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock); + } + /* now we're locked, new peers can't grab it anymore, + * existing ones already have the ref_cnt. + */ + if (HA_ATOMIC_LOAD(&ts->ref_cnt)) + continue; + } + /* session expired, trash it */ ebmb_delete(&ts->key); - if (ts->upd.node.leaf_p) { - HA_RWLOCK_WRLOCK(STK_TABLE_LOCK, &t->updt_lock); - eb32_delete(&ts->upd); - HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock); - } + eb32_delete(&ts->upd); __stksess_free(t, ts); batched++; done_per_shard++; } + if (updt_locked) + HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->updt_lock); + HA_RWLOCK_WRUNLOCK(STK_TABLE_LOCK, &t->shards[shard].sh_lock); if (max_search <= 0)