Merge branch 'ps/cache-tree-w-broken-index-entry'

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
This commit is contained in:
Taylor Blau
2024-10-22 14:40:38 -04:00
5 changed files with 97 additions and 43 deletions

View File

@@ -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;
}