mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-03-31 07:37:54 +00:00
BUG/MAJOR: cache: fix random crashes caused by incorrect delete() on non-first blocks
Several segfaults were reported in the cache, each time in eb_delete() called from cache_free_blocks() itself called from shctx_row_reserve_hot(). Each time the tree node was corrupted with random cached data (often JS or HTML contents). The problem comes from an incompatibility between the cache's expectations and the recycling algorithm used in the shctx. The shctx allocates and releases a chain of blocks at once. And when it needs to allocate N blocks from the avail list while a chain of M>N is found, it picks the first N from the list, moves them to the hot list, and marks all remaining M-N blocks as isolated blocks (chains of 1). For each such released block, the shctx->free_block() callback is used and passed a pointer to the first and current block of the chain. For the cache, it's cache_free_blocks(). What this function does is check that the current block is the first one, and in this case delete the object from the tree and mark it as not in tree by setting key to zero. The problem this causes is that the tail blocks when M>N become first blocks for the next call to shctx_row_reserve_hot(), these ones will be passed to cache_free_blocks() as list heads, and will be sent to eb_delete() despite containing only cached data. The simplest solution for now is to mark each block as holding no cache object by setting key to zero all the time. It keeps the principle used elsewhere in the code. The SSL code is not subject to this problem because it relies on the block's len not being null, which happens immediately after a block was released. It was uncertain however whether this method is suitable for the cache. It is not critical though since this code is going to change soon in 1.9 to dynamically allocate only the number of required blocks. This fix must be backported to 1.8. Thanks to Thierry for providing exploitable cores.
This commit is contained in:
parent
afe1de5d98
commit
5bd37fa625
12
src/cache.c
12
src/cache.c
@ -378,13 +378,11 @@ int http_calc_maxage(struct stream *s, struct cache *cache)
|
||||
|
||||
static void cache_free_blocks(struct shared_block *first, struct shared_block *block)
|
||||
{
|
||||
if (first == block) {
|
||||
struct cache_entry *object = (struct cache_entry *)first->data;
|
||||
if (object->eb.key) {
|
||||
eb32_delete(&object->eb);
|
||||
object->eb.key = 0;
|
||||
}
|
||||
}
|
||||
struct cache_entry *object = (struct cache_entry *)block->data;
|
||||
|
||||
if (first == block && object->eb.key)
|
||||
eb32_delete(&object->eb);
|
||||
object->eb.key = 0;
|
||||
}
|
||||
|
||||
/*
|
||||
|
Loading…
Reference in New Issue
Block a user