aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2024-11-20 14:42:58 +0900
committerJunio C Hamano <gitster@pobox.com>2024-11-20 14:42:59 +0900
commit257f2de9644536964c02eed53271df89e132d4f4 (patch)
treeaab2dc6fa6d86b450689a87bd892d5d27f5b27c2
parent76c1953395770e5334a506eb59bc19ff4cd99e6c (diff)
parentecb5c4318cd8b5bbb72f943b6128fe946ffcb56e (diff)
downloadgit-257f2de9644536964c02eed53271df89e132d4f4.tar.gz
Merge branch 'ps/cache-tree-w-broken-index-entry' into maint-2.47
Fail gracefully instead of crashing when attempting to write the contents of a corrupt in-core index as a tree object. * ps/cache-tree-w-broken-index-entry: unpack-trees: detect mismatching number of cache-tree/index entries cache-tree: detect mismatching number of index entries cache-tree: refactor verification to return error codes
Diffstat
-rw-r--r--cache-tree.c102
-rw-r--r--cache-tree.h2
-rw-r--r--read-cache.c5
-rwxr-xr-xt/t4058-diff-duplicates.sh19
-rw-r--r--unpack-trees.c12
5 files changed, 97 insertions, 43 deletions
diff --git a/cache-tree.c b/cache-tree.c
index b482167a69..c595e86120 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -1,6 +1,7 @@
#define USE_THE_REPOSITORY_VARIABLE
#include "git-compat-util.h"
+#include "gettext.h"
#include "hex.h"
#include "lockfile.h"
#include "tree.h"
@@ -865,15 +866,15 @@ int cache_tree_matches_traversal(struct cache_tree *root,
return 0;
}
-static void verify_one_sparse(struct index_state *istate,
- struct strbuf *path,
- int pos)
+static int verify_one_sparse(struct index_state *istate,
+ struct strbuf *path,
+ int pos)
{
struct cache_entry *ce = istate->cache[pos];
-
if (!S_ISSPARSEDIR(ce->ce_mode))
- BUG("directory '%s' is present in index, but not sparse",
- path->buf);
+ return error(_("directory '%s' is present in index, but not sparse"),
+ path->buf);
+ return 0;
}
/*
@@ -882,6 +883,7 @@ static void verify_one_sparse(struct index_state *istate,
* 1 - Restart verification - a call to ensure_full_index() freed the cache
* tree that is being verified and verification needs to be restarted from
* the new toplevel cache tree.
+ * -1 - Verification failed.
*/
static int verify_one(struct repository *r,
struct index_state *istate,
@@ -891,18 +893,23 @@ static int verify_one(struct repository *r,
int i, pos, len = path->len;
struct strbuf tree_buf = STRBUF_INIT;
struct object_id new_oid;
+ int ret;
for (i = 0; i < it->subtree_nr; i++) {
strbuf_addf(path, "%s/", it->down[i]->name);
- if (verify_one(r, istate, it->down[i]->cache_tree, path))
- return 1;
+ ret = verify_one(r, istate, it->down[i]->cache_tree, path);
+ if (ret)
+ goto out;
+
strbuf_setlen(path, len);
}
if (it->entry_count < 0 ||
/* no verification on tests (t7003) that replace trees */
- lookup_replace_object(r, &it->oid) != &it->oid)
- return 0;
+ lookup_replace_object(r, &it->oid) != &it->oid) {
+ ret = 0;
+ goto out;
+ }
if (path->len) {
/*
@@ -912,12 +919,14 @@ static int verify_one(struct repository *r,
*/
int is_sparse = istate->sparse_index;
pos = index_name_pos(istate, path->buf, path->len);
- if (is_sparse && !istate->sparse_index)
- return 1;
+ if (is_sparse && !istate->sparse_index) {
+ ret = 1;
+ goto out;
+ }
if (pos >= 0) {
- verify_one_sparse(istate, path, pos);
- return 0;
+ ret = verify_one_sparse(istate, path, pos);
+ goto out;
}
pos = -pos - 1;
@@ -925,6 +934,11 @@ static int verify_one(struct repository *r,
pos = 0;
}
+ if (it->entry_count + pos > istate->cache_nr) {
+ ret = error(_("corrupted cache-tree has entries not present in index"));
+ goto out;
+ }
+
i = 0;
while (i < it->entry_count) {
struct cache_entry *ce = istate->cache[pos + i];
@@ -935,16 +949,23 @@ static int verify_one(struct repository *r,
unsigned mode;
int entlen;
- if (ce->ce_flags & (CE_STAGEMASK | CE_INTENT_TO_ADD | CE_REMOVE))
- BUG("%s with flags 0x%x should not be in cache-tree",
- ce->name, ce->ce_flags);
+ if (ce->ce_flags & (CE_STAGEMASK | CE_INTENT_TO_ADD | CE_REMOVE)) {
+ ret = error(_("%s with flags 0x%x should not be in cache-tree"),
+ ce->name, ce->ce_flags);
+ goto out;
+ }
+
name = ce->name + path->len;
slash = strchr(name, '/');
if (slash) {
entlen = slash - name;
+
sub = find_subtree(it, ce->name + path->len, entlen, 0);
- if (!sub || sub->cache_tree->entry_count < 0)
- BUG("bad subtree '%.*s'", entlen, name);
+ if (!sub || sub->cache_tree->entry_count < 0) {
+ ret = error(_("bad subtree '%.*s'"), entlen, name);
+ goto out;
+ }
+
oid = &sub->cache_tree->oid;
mode = S_IFDIR;
i += sub->cache_tree->entry_count;
@@ -957,27 +978,50 @@ static int verify_one(struct repository *r,
strbuf_addf(&tree_buf, "%o %.*s%c", mode, entlen, name, '\0');
strbuf_add(&tree_buf, oid->hash, r->hash_algo->rawsz);
}
+
hash_object_file(r->hash_algo, tree_buf.buf, tree_buf.len, OBJ_TREE,
&new_oid);
- if (!oideq(&new_oid, &it->oid))
- BUG("cache-tree for path %.*s does not match. "
- "Expected %s got %s", len, path->buf,
- oid_to_hex(&new_oid), oid_to_hex(&it->oid));
+
+ if (!oideq(&new_oid, &it->oid)) {
+ ret = error(_("cache-tree for path %.*s does not match. "
+ "Expected %s got %s"), len, path->buf,
+ oid_to_hex(&new_oid), oid_to_hex(&it->oid));
+ goto out;
+ }
+
+ ret = 0;
+out:
strbuf_setlen(path, len);
strbuf_release(&tree_buf);
- return 0;
+ return ret;
}
-void cache_tree_verify(struct repository *r, struct index_state *istate)
+int cache_tree_verify(struct repository *r, struct index_state *istate)
{
struct strbuf path = STRBUF_INIT;
+ int ret;
- if (!istate->cache_tree)
- return;
- if (verify_one(r, istate, istate->cache_tree, &path)) {
+ if (!istate->cache_tree) {
+ ret = 0;
+ goto out;
+ }
+
+ ret = verify_one(r, istate, istate->cache_tree, &path);
+ if (ret < 0)
+ goto out;
+ if (ret > 0) {
strbuf_reset(&path);
- if (verify_one(r, istate, istate->cache_tree, &path))
+
+ ret = verify_one(r, istate, istate->cache_tree, &path);
+ if (ret < 0)
+ goto out;
+ if (ret > 0)
BUG("ensure_full_index() called twice while verifying cache tree");
}
+
+ ret = 0;
+
+out:
strbuf_release(&path);
+ return ret;
}
diff --git a/cache-tree.h b/cache-tree.h
index faae88be63..b82c4963e7 100644
--- a/cache-tree.h
+++ b/cache-tree.h
@@ -33,7 +33,7 @@ struct cache_tree *cache_tree_read(const char *buffer, unsigned long size);
int cache_tree_fully_valid(struct cache_tree *);
int cache_tree_update(struct index_state *, int);
-void cache_tree_verify(struct repository *, struct index_state *);
+int cache_tree_verify(struct repository *, struct index_state *);
/* bitmasks to write_index_as_tree flags */
#define WRITE_TREE_MISSING_OK 1
diff --git a/read-cache.c b/read-cache.c
index 3c078afadb..294971a5f4 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -3334,8 +3334,9 @@ int write_locked_index(struct index_state *istate, struct lock_file *lock,
int new_shared_index, ret, test_split_index_env;
struct split_index *si = istate->split_index;
- if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0))
- cache_tree_verify(the_repository, istate);
+ if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0) &&
+ cache_tree_verify(the_repository, istate) < 0)
+ return -1;
if ((flags & SKIP_IF_UNCHANGED) && !istate->cache_changed) {
if (flags & COMMIT_LOCK)
diff --git a/t/t4058-diff-duplicates.sh b/t/t4058-diff-duplicates.sh
index 2501c89c1c..18e5ac88c3 100755
--- a/t/t4058-diff-duplicates.sh
+++ b/t/t4058-diff-duplicates.sh
@@ -10,6 +10,8 @@
# that the diff output isn't wildly unreasonable.
test_description='test tree diff when trees have duplicate entries'
+
+TEST_PASSES_SANITIZE_LEAK=true
. ./test-lib.sh
# make_tree_entry <mode> <mode> <sha1>
@@ -132,22 +134,23 @@ test_expect_success 'create a few commits' '
rm commit_id up final
'
-test_expect_failure 'git read-tree does not segfault' '
- test_when_finished rm .git/index.lock &&
- test_might_fail git read-tree --reset base
+test_expect_success 'git read-tree does not segfault' '
+ test_must_fail git read-tree --reset base 2>err &&
+ test_grep "error: corrupted cache-tree has entries not present in index" err
'
-test_expect_failure 'reset --hard does not segfault' '
- test_when_finished rm .git/index.lock &&
+test_expect_success 'reset --hard does not segfault' '
git checkout base &&
- test_might_fail git reset --hard
+ test_must_fail git reset --hard 2>err &&
+ test_grep "error: corrupted cache-tree has entries not present in index" err
'
-test_expect_failure 'git diff HEAD does not segfault' '
+test_expect_success 'git diff HEAD does not segfault' '
git checkout base &&
GIT_TEST_CHECK_CACHE_TREE=false &&
git reset --hard &&
- test_might_fail git diff HEAD
+ test_must_fail git diff HEAD 2>err &&
+ test_grep "error: corrupted cache-tree has entries not present in index" err
'
test_expect_failure 'can switch to another branch when status is empty' '
diff --git a/unpack-trees.c b/unpack-trees.c
index 9a55cb6204..e10a9d1209 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -808,6 +808,8 @@ static int traverse_by_cache_tree(int pos, int nr_entries, int nr_names,
if (!o->merge)
BUG("We need cache-tree to do this optimization");
+ if (nr_entries + pos > o->src_index->cache_nr)
+ return error(_("corrupted cache-tree has entries not present in index"));
/*
* Do what unpack_callback() and unpack_single_entry() normally
@@ -2070,9 +2072,13 @@ int unpack_trees(unsigned len, struct tree_desc *t, struct unpack_trees_options
if (o->dst_index) {
move_index_extensions(&o->internal.result, o->src_index);
if (!ret) {
- if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0))
- cache_tree_verify(the_repository,
- &o->internal.result);
+ if (git_env_bool("GIT_TEST_CHECK_CACHE_TREE", 0) &&
+ cache_tree_verify(the_repository,
+ &o->internal.result) < 0) {
+ ret = -1;
+ goto done;
+ }
+
if (!o->skip_cache_tree_update &&
!cache_tree_fully_valid(o->internal.result.cache_tree))
cache_tree_update(&o->internal.result,
generated by cgit 1.2.3-korg (git 2.43.0) at 2025-12-06 19:07:41 +0000