BUG/MAJOR: cache: Crash because of wrong cache entry deleted

When "vary" is enabled, we can have multiple entries for a given primary
key in the cache tree. There is a limit to how many secondary entries
can be inserted for a given key. When we try to insert a new secondary
entry, if the limit is already reached, we can try to find expired
entries with the same primary key, and if the limit is still reached we
want to abort the current insertion and to remove the node that was just
inserted.

In commit "a29b073: MEDIUM: cache: Add refcount on cache_entry" though,
a regression was introduced. Instead of removing the entry just inserted
as the comments suggested, we removed the second to last entry and
returned NULL. We then reset the eb.key of the cache_entry in the caller
because we assumed that the entry was already removed from the tree.

This means that some entries with an empty key were wrongly kept in the
tree and the last secondary entry, which keeps the number of secondary
entries of a given key was removed.

This ended up causing some crashes later on when we tried to iterate
over the elements of this given key. The crash could occur in multiple
places, either when trying to retrieve an entry or to add some new ones.

This crash was raised in GitHub issue #2950.
The fix should be backported up to 3.0.

diff --git a/src/cache.c b/src/cache.c
index c2b374933ef0b5a3f36627e991c2c02863f4c1c6..4ef4bc7c3342fd3602bd5086fe2bb0d124948d16 100644 (file)
--- a/src/cache.c
+++ b/src/cache.c
@@ -493,7 +493,7 @@ static struct eb32_node *insert_entry(struct cache *cache, struct cache_tree *tr
                        if (last_clear_ts == date.tv_sec) {
                                /* Too many entries for this primary key, clear the
                                 * one that was inserted. */
-                               release_entry_locked(tree, entry);
+                               release_entry_locked(tree, new_entry);
                                return NULL;
                        }
 
@@ -503,7 +503,7 @@ static struct eb32_node *insert_entry(struct cache *cache, struct cache_tree *tr
                                 * the newly inserted one. */
                                entry = container_of(prev, struct cache_entry, eb);
                                entry->last_clear_ts = date.tv_sec;
-                               release_entry_locked(tree, entry);
+                               release_entry_locked(tree, new_entry);
                                return NULL;
                        }
                }