pack: fix race in pack_entry_find_offset

In `pack_entry_find_offset`, we try to find the offset of a
certain object in the pack file. To do so, we first assert if the
packfile has already been opened and open it if not. Opening the
packfile is guarded with a mutex, so concurrent access to this is
in fact safe.

What is not thread-safe though is our calculation of offsets
inside the packfile. Assume two threads calling
`pack_entry_find_offset` at the same time. We first calculate the
offset and index location and only then determine if the pack has
already been opened. If so, we re-calculate the offset and index
address.

Now the case for two threads: thread 1 first calculates the
addresses and is subsequently suspended. The second thread will
now call `pack_index_open` and initialize the pack file,
calculating its addresses correctly. When the first thread is
resumed now, he'll see that the pack file has already been
initialized and will happily proceed with the addresses it has
already calculated before the check. As the pack file was not
initialized before, these addresses are bogus.

Fix the issue by only calculating the addresses after having
checked if the pack file is open.
This commit is contained in:
Patrick Steinhardt 2016-11-02 12:23:12 +01:00
parent 19001ca7ba
commit 0cf15e39f3

View File

@ -1268,8 +1268,8 @@ static int pack_entry_find_offset(
const git_oid *short_oid,
size_t len)
{
const uint32_t *level1_ofs = p->index_map.data;
const unsigned char *index = p->index_map.data;
const uint32_t *level1_ofs;
const unsigned char *index;
unsigned hi, lo, stride;
int pos, found = 0;
git_off_t offset;
@ -1283,10 +1283,10 @@ static int pack_entry_find_offset(
if ((error = pack_index_open(p)) < 0)
return error;
assert(p->index_map.data);
}
index = p->index_map.data;
level1_ofs = p->index_map.data;
}
if (p->index_version > 1) {
level1_ofs += 2;