Skip to content

Commit e058b18

Browse files
committed
Merge branch 'pw/sparse-cache-tree-verify-fix'
Recent sparse-index addition, namely any use of index_name_pos(), can expand sparse index entries and breaks any code that walks cache-tree or existing index entries. One such instance of such a breakage has been corrected. * pw/sparse-cache-tree-verify-fix: t1092: run "rebase --apply" without "-q" in testing sparse index: fix use-after-free bug in cache_tree_verify()
2 parents 2c428e4 + 5e311ed commit e058b18

File tree

2 files changed

+30
-9
lines changed

2 files changed

+30
-9
lines changed

cache-tree.c

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -827,32 +827,48 @@ static void verify_one_sparse(struct repository *r,
827827
path->buf);
828828
}
829829

830-
static void verify_one(struct repository *r,
831-
struct index_state *istate,
832-
struct cache_tree *it,
833-
struct strbuf *path)
830+
/*
831+
* Returns:
832+
* 0 - Verification completed.
833+
* 1 - Restart verification - a call to ensure_full_index() freed the cache
834+
* tree that is being verified and verification needs to be restarted from
835+
* the new toplevel cache tree.
836+
*/
837+
static int verify_one(struct repository *r,
838+
struct index_state *istate,
839+
struct cache_tree *it,
840+
struct strbuf *path)
834841
{
835842
int i, pos, len = path->len;
836843
struct strbuf tree_buf = STRBUF_INIT;
837844
struct object_id new_oid;
838845

839846
for (i = 0; i < it->subtree_nr; i++) {
840847
strbuf_addf(path, "%s/", it->down[i]->name);
841-
verify_one(r, istate, it->down[i]->cache_tree, path);
848+
if (verify_one(r, istate, it->down[i]->cache_tree, path))
849+
return 1;
842850
strbuf_setlen(path, len);
843851
}
844852

845853
if (it->entry_count < 0 ||
846854
/* no verification on tests (t7003) that replace trees */
847855
lookup_replace_object(r, &it->oid) != &it->oid)
848-
return;
856+
return 0;
849857

850858
if (path->len) {
859+
/*
860+
* If the index is sparse and the cache tree is not
861+
* index_name_pos() may trigger ensure_full_index() which will
862+
* free the tree that is being verified.
863+
*/
864+
int is_sparse = istate->sparse_index;
851865
pos = index_name_pos(istate, path->buf, path->len);
866+
if (is_sparse && !istate->sparse_index)
867+
return 1;
852868

853869
if (pos >= 0) {
854870
verify_one_sparse(r, istate, it, path, pos);
855-
return;
871+
return 0;
856872
}
857873

858874
pos = -pos - 1;
@@ -900,6 +916,7 @@ static void verify_one(struct repository *r,
900916
oid_to_hex(&new_oid), oid_to_hex(&it->oid));
901917
strbuf_setlen(path, len);
902918
strbuf_release(&tree_buf);
919+
return 0;
903920
}
904921

905922
void cache_tree_verify(struct repository *r, struct index_state *istate)
@@ -908,6 +925,10 @@ void cache_tree_verify(struct repository *r, struct index_state *istate)
908925

909926
if (!istate->cache_tree)
910927
return;
911-
verify_one(r, istate, istate->cache_tree, &path);
928+
if (verify_one(r, istate, istate->cache_tree, &path)) {
929+
strbuf_reset(&path);
930+
if (verify_one(r, istate, istate->cache_tree, &path))
931+
BUG("ensure_full_index() called twice while verifying cache tree");
932+
}
912933
strbuf_release(&path);
913934
}

t/t1092-sparse-checkout-compatibility.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ test_expect_success 'checkout and reset (mixed) [sparse]' '
514514
test_expect_success 'merge, cherry-pick, and rebase' '
515515
init_repos &&
516516
517-
for OPERATION in "merge -m merge" cherry-pick rebase
517+
for OPERATION in "merge -m merge" cherry-pick "rebase --apply" "rebase --merge"
518518
do
519519
test_all_match git checkout -B temp update-deep &&
520520
test_all_match git $OPERATION update-folder1 &&

0 commit comments

Comments
 (0)