thodg/libgit2

2016-11-02T12:23:12

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.

diff --git a/src/pack.c b/src/pack.c
index 310f00f..2ee0c60 100644
--- a/src/pack.c
+++ b/src/pack.c
@@ -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,11 +1283,11 @@ 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;
 	}
 
+	index = p->index_map.data;
+	level1_ofs = p->index_map.data;
+
 	if (p->index_version > 1) {
 		level1_ofs += 2;
 		index += 8;