From c0e302fe79a127e7093a14757351e85cfba20b24 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Sat, 23 May 2026 22:17:10 +0200 Subject: [PATCH] BUG/MINOR: dict: fix refcount race on insert collision In dict_insert(), when ebis_insert() returns an existing node n indicating that another thread inserted the same key concurrently, the code freed its own newly-allocated entry and returned the winner without bumping its refcount. Both callers then held a reference with refcount=1 instead of 2, so when one expires the other becomes a use-after-free or double-free. The bug likely comes from the fact that new_dict_entry() creates an entry with a refcount preset to 1 (saves an atomic op) and that because of this there is no refcount increment upon a successful insertion in the tree, resulting in requiring different code paths for collision and normal insertion. A simple fix consists in bumping the refcount under the lock and unlocking only at the end, but this would mean performing two free() calls under a lock, which we always try to avoid. The code was slightly rearranged so that we can now bump the existing entry's refcount under the lock in case of duplicate, or unlock immediately in the common case, so that the free() call is done out of the lock. The probably of the race is very low (at peers connection setup only), reason why it's marked low. This should be backported to all versions. --- src/dict.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/dict.c b/src/dict.c index 34689ef77..fc6bdad22 100644 --- a/src/dict.c +++ b/src/dict.c @@ -79,7 +79,7 @@ static struct dict_entry *__dict_lookup(struct dict *d, const char *s) */ struct dict_entry *dict_insert(struct dict *d, char *s) { - struct dict_entry *de; + struct dict_entry *de, *tree_de; struct ebpt_node *n; HA_RWLOCK_RDLOCK(DICT_LOCK, &d->rwlock); @@ -97,13 +97,18 @@ struct dict_entry *dict_insert(struct dict *d, char *s) HA_RWLOCK_WRLOCK(DICT_LOCK, &d->rwlock); n = ebis_insert(&d->values, &de->value); - HA_RWLOCK_WRUNLOCK(DICT_LOCK, &d->rwlock); - if (n != &de->value) { + tree_de = container_of(n, struct dict_entry, value); + if (tree_de == de) + HA_RWLOCK_WRUNLOCK(DICT_LOCK, &d->rwlock); + else { + /* another entry was already there, we'll return it, kill + * ours and bump the other's refcount before returning it. + */ + HA_ATOMIC_INC(&tree_de->refcount); + HA_RWLOCK_WRUNLOCK(DICT_LOCK, &d->rwlock); free_dict_entry(de); - de = container_of(n, struct dict_entry, value); } - - return de; + return tree_de; }