upstream/haproxy
1
0
Fork 0
mirror of https://github.com/haproxy/haproxy.git synced 2026-05-27 11:52:34 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 9

BUG/MINOR: dict: fix refcount race on insert collision

Browse source 
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.
This commit is contained in:
Willy Tarreau 2026-05-23 22:17:10 +02:00
parent c1aad1ab1c
commit c7c71cb5d2
1 changed files with 11 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/dict.c
View file

@ -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;
}