Commit 5f32c506 by Edward Thomson Committed by Edward Thomson

racy: make git_index_read_index handle raciness

Ensure that `git_index_read_index` clears the uptodate bit on
files that it modifies.

Further, do not propagate the cache from an on-disk index into
another on-disk index.  Although this should not be done, as
`git_index_read_index` is used to bring an in-memory index into
another index (that may or may not be on-disk), ensure that we do
not accidentally bring in these bits when misused.
parent c30051f0
...@@ -1003,20 +1003,11 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out, ...@@ -1003,20 +1003,11 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out,
static void index_entry_cpy( static void index_entry_cpy(
git_index_entry *tgt, git_index_entry *tgt,
git_index *index, const git_index_entry *src)
const git_index_entry *src,
bool update_path)
{ {
const char *tgt_path = tgt->path; const char *tgt_path = tgt->path;
memcpy(tgt, src, sizeof(*tgt)); memcpy(tgt, src, sizeof(*tgt));
/* keep the existing path buffer, but update the path to the one
* given by the caller, if we trust it.
*/
tgt->path = tgt_path; tgt->path = tgt_path;
if (index->ignore_case && update_path)
memcpy((char *)tgt->path, src->path, strlen(tgt->path));
} }
static int index_entry_dup( static int index_entry_dup(
...@@ -1024,18 +1015,32 @@ static int index_entry_dup( ...@@ -1024,18 +1015,32 @@ static int index_entry_dup(
git_index *index, git_index *index,
const git_index_entry *src) const git_index_entry *src)
{ {
git_index_entry *entry; if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0)
return -1;
if (!src) { index_entry_cpy(*out, src);
*out = NULL; return 0;
return 0; }
}
if (index_entry_create(&entry, INDEX_OWNER(index), src->path) < 0) static void index_entry_cpy_nocache(
git_index_entry *tgt,
const git_index_entry *src)
{
git_oid_cpy(&tgt->id, &src->id);
tgt->mode = src->mode;
tgt->flags = src->flags;
tgt->flags_extended = (src->flags_extended & GIT_IDXENTRY_EXTENDED_FLAGS);
}
static int index_entry_dup_nocache(
git_index_entry **out,
git_index *index,
const git_index_entry *src)
{
if (index_entry_create(out, INDEX_OWNER(index), src->path) < 0)
return -1; return -1;
index_entry_cpy(entry, index, src, false); index_entry_cpy_nocache(*out, src);
*out = entry;
return 0; return 0;
} }
...@@ -1331,8 +1336,13 @@ static int index_insert( ...@@ -1331,8 +1336,13 @@ static int index_insert(
* and return it in place of the passed in one. * and return it in place of the passed in one.
*/ */
else if (existing) { else if (existing) {
if (replace) if (replace) {
index_entry_cpy(existing, index, entry, trust_path); index_entry_cpy(existing, entry);
if (trust_path)
memcpy((char *)existing->path, entry->path, strlen(entry->path));
}
index_entry_free(entry); index_entry_free(entry);
*entry_ptr = entry = existing; *entry_ptr = entry = existing;
} }
...@@ -1709,9 +1719,12 @@ int git_index_conflict_add(git_index *index, ...@@ -1709,9 +1719,12 @@ int git_index_conflict_add(git_index *index,
assert (index); assert (index);
if ((ret = index_entry_dup(&entries[0], index, ancestor_entry)) < 0 || if ((ancestor_entry &&
(ret = index_entry_dup(&entries[1], index, our_entry)) < 0 || (ret = index_entry_dup(&entries[0], index, ancestor_entry)) < 0) ||
(ret = index_entry_dup(&entries[2], index, their_entry)) < 0) (our_entry &&
(ret = index_entry_dup(&entries[1], index, our_entry)) < 0) ||
(their_entry &&
(ret = index_entry_dup(&entries[2], index, their_entry)) < 0))
goto on_error; goto on_error;
/* Validate entries */ /* Validate entries */
...@@ -2849,7 +2862,7 @@ static int read_tree_cb( ...@@ -2849,7 +2862,7 @@ static int read_tree_cb(
entry->mode == old_entry->mode && entry->mode == old_entry->mode &&
git_oid_equal(&entry->id, &old_entry->id)) git_oid_equal(&entry->id, &old_entry->id))
{ {
index_entry_cpy(entry, data->index, old_entry, false); index_entry_cpy(entry, old_entry);
entry->flags_extended = 0; entry->flags_extended = 0;
} }
...@@ -2974,7 +2987,10 @@ int git_index_read_index( ...@@ -2974,7 +2987,10 @@ int git_index_read_index(
goto done; goto done;
while (true) { while (true) {
git_index_entry *add_entry = NULL, *remove_entry = NULL; git_index_entry
*dup_entry = NULL,
*add_entry = NULL,
*remove_entry = NULL;
int diff; int diff;
if (old_entry && new_entry) if (old_entry && new_entry)
...@@ -2989,8 +3005,7 @@ int git_index_read_index( ...@@ -2989,8 +3005,7 @@ int git_index_read_index(
if (diff < 0) { if (diff < 0) {
remove_entry = (git_index_entry *)old_entry; remove_entry = (git_index_entry *)old_entry;
} else if (diff > 0) { } else if (diff > 0) {
if ((error = index_entry_dup(&add_entry, index, new_entry)) < 0) dup_entry = (git_index_entry *)new_entry;
goto done;
} else { } else {
/* Path and stage are equal, if the OID is equal, keep it to /* Path and stage are equal, if the OID is equal, keep it to
* keep the stat cache data. * keep the stat cache data.
...@@ -2998,13 +3013,16 @@ int git_index_read_index( ...@@ -2998,13 +3013,16 @@ int git_index_read_index(
if (git_oid_equal(&old_entry->id, &new_entry->id)) { if (git_oid_equal(&old_entry->id, &new_entry->id)) {
add_entry = (git_index_entry *)old_entry; add_entry = (git_index_entry *)old_entry;
} else { } else {
if ((error = index_entry_dup(&add_entry, index, new_entry)) < 0) dup_entry = (git_index_entry *)new_entry;
goto done;
remove_entry = (git_index_entry *)old_entry; remove_entry = (git_index_entry *)old_entry;
} }
} }
if (dup_entry) {
if ((error = index_entry_dup_nocache(&add_entry, index, dup_entry)) < 0)
goto done;
}
if (add_entry) { if (add_entry) {
if ((error = git_vector_insert(&new_entries, add_entry)) == 0) if ((error = git_vector_insert(&new_entries, add_entry)) == 0)
INSERT_IN_MAP_EX(index, new_entries_map, add_entry, error); INSERT_IN_MAP_EX(index, new_entries_map, add_entry, error);
......
...@@ -278,3 +278,51 @@ void test_index_racy__read_tree_clears_uptodate_bit(void) ...@@ -278,3 +278,51 @@ void test_index_racy__read_tree_clears_uptodate_bit(void)
git_tree_free(tree); git_tree_free(tree);
git_index_free(index); git_index_free(index);
} }
void test_index_racy__read_index_smudges(void)
{
git_index *index, *newindex;
const git_index_entry *entry;
/* if we are reading an index into our new index, ensure that any
* racy entries in the index that we're reading are smudged so that
* we don't propagate their timestamps without further investigation.
*/
setup_race();
cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_new(&newindex));
cl_git_pass(git_index_read_index(newindex, index));
cl_assert(entry = git_index_get_bypath(newindex, "A", 0));
cl_assert_equal_i(0, entry->file_size);
git_index_free(index);
git_index_free(newindex);
}
void test_index_racy__read_index_clears_uptodate_bit(void)
{
git_index *index, *newindex;
const git_index_entry *entry;
git_oid id;
setup_uptodate_files();
cl_git_pass(git_repository_index(&index, g_repo));
cl_git_pass(git_index_new(&newindex));
cl_git_pass(git_index_read_index(newindex, index));
/* ensure that files brought in from the other index are not uptodate */
cl_assert((entry = git_index_get_bypath(newindex, "A", 0)));
cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE));
cl_assert((entry = git_index_get_bypath(newindex, "B", 0)));
cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE));
cl_assert((entry = git_index_get_bypath(newindex, "C", 0)));
cl_assert_equal_i(0, (entry->flags_extended & GIT_IDXENTRY_UPTODATE));
git_index_free(index);
git_index_free(newindex);
}
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