Commit d884d1c4 by Jason Haslam Committed by Carlos Martín Nieto

Fix race in git_packfile_unpack.

Increment refcount of newly added cache entries just like existing
entries looked up from the cache. Otherwise the new entry can be
evicted from the cache and destroyed while it's still in use.
parent 277d6477
...@@ -56,6 +56,7 @@ static git_pack_cache_entry *new_cache_object(git_rawobj *source) ...@@ -56,6 +56,7 @@ static git_pack_cache_entry *new_cache_object(git_rawobj *source)
if (!e) if (!e)
return NULL; return NULL;
git_atomic_inc(&e->refcount);
memcpy(&e->raw, source, sizeof(git_rawobj)); memcpy(&e->raw, source, sizeof(git_rawobj));
return e; return e;
...@@ -145,7 +146,11 @@ static void free_lowest_entry(git_pack_cache *cache) ...@@ -145,7 +146,11 @@ static void free_lowest_entry(git_pack_cache *cache)
} }
} }
static int cache_add(git_pack_cache *cache, git_rawobj *base, git_off_t offset) static int cache_add(
git_pack_cache_entry **cached_out,
git_pack_cache *cache,
git_rawobj *base,
git_off_t offset)
{ {
git_pack_cache_entry *entry; git_pack_cache_entry *entry;
int error, exists = 0; int error, exists = 0;
...@@ -171,6 +176,8 @@ static int cache_add(git_pack_cache *cache, git_rawobj *base, git_off_t offset) ...@@ -171,6 +176,8 @@ static int cache_add(git_pack_cache *cache, git_rawobj *base, git_off_t offset)
assert(error != 0); assert(error != 0);
kh_value(cache->entries, k) = entry; kh_value(cache->entries, k) = entry;
cache->memory_used += entry->raw.len; cache->memory_used += entry->raw.len;
*cached_out = entry;
} }
git_mutex_unlock(&cache->lock); git_mutex_unlock(&cache->lock);
/* Somebody beat us to adding it into the cache */ /* Somebody beat us to adding it into the cache */
...@@ -699,7 +706,7 @@ int git_packfile_unpack( ...@@ -699,7 +706,7 @@ int git_packfile_unpack(
* long as it's not already the cached one. * long as it's not already the cached one.
*/ */
if (!cached) if (!cached)
free_base = !!cache_add(&p->bases, obj, elem->base_key); free_base = !!cache_add(&cached, &p->bases, obj, elem->base_key);
elem = &stack[elem_pos - 1]; elem = &stack[elem_pos - 1];
curpos = elem->offset; curpos = elem->offset;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment