diff options
| -rw-r--r-- | builtin/fsck.c | 36 | ||||
| -rw-r--r-- | pack-bitmap.c | 4 | ||||
| -rw-r--r-- | pack-revindex.c | 43 | ||||
| -rw-r--r-- | pack-revindex.h | 16 | ||||
| -rwxr-xr-x | t/t5325-reverse-index.sh | 74 |
5 files changed, 169 insertions, 4 deletions
diff --git a/builtin/fsck.c b/builtin/fsck.c index 35a6de3cdb..2cd461b84c 100644 --- a/builtin/fsck.c +++ b/builtin/fsck.c @@ -26,6 +26,7 @@ #include "resolve-undo.h" #include "run-command.h" #include "worktree.h" +#include "pack-revindex.h" #define REACHABLE 0x0001 #define SEEN 0x0002 @@ -55,6 +56,7 @@ static int name_objects; #define ERROR_REFS 010 #define ERROR_COMMIT_GRAPH 020 #define ERROR_MULTI_PACK_INDEX 040 +#define ERROR_PACK_REV_INDEX 0100 static const char *describe_object(const struct object_id *oid) { @@ -858,6 +860,38 @@ static int mark_packed_for_connectivity(const struct object_id *oid, return 0; } +static int check_pack_rev_indexes(struct repository *r, int show_progress) +{ + struct progress *progress = NULL; + uint32_t pack_count = 0; + int res = 0; + + if (show_progress) { + for (struct packed_git *p = get_all_packs(the_repository); p; p = p->next) + pack_count++; + progress = start_delayed_progress("Verifying reverse pack-indexes", pack_count); + pack_count = 0; + } + + for (struct packed_git *p = get_all_packs(the_repository); p; p = p->next) { + int load_error = load_pack_revindex_from_disk(p); + + if (load_error < 0) { + error(_("unable to load rev-index for pack '%s'"), p->pack_name); + res = ERROR_PACK_REV_INDEX; + } else if (!load_error && + !load_pack_revindex(the_repository, p) && + verify_pack_revindex(p)) { + error(_("invalid rev-index for pack '%s'"), p->pack_name); + res = ERROR_PACK_REV_INDEX; + } + display_progress(progress, ++pack_count); + } + stop_progress(&progress); + + return res; +} + static char const * const fsck_usage[] = { N_("git fsck [--tags] [--root] [--unreachable] [--cache] [--no-reflogs]\n" " [--[no-]full] [--strict] [--verbose] [--lost-found]\n" @@ -1021,6 +1055,8 @@ int cmd_fsck(int argc, const char **argv, const char *prefix) free_worktrees(worktrees); } + errors_found |= check_pack_rev_indexes(the_repository, show_progress); + check_connectivity(); if (the_repository->settings.core_commit_graph) { diff --git a/pack-bitmap.c b/pack-bitmap.c index cd5f472488..e0fad723bf 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -381,7 +381,7 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git, goto cleanup; } - if (load_midx_revindex(bitmap_git->midx) < 0) { + if (load_midx_revindex(bitmap_git->midx)) { warning(_("multi-pack bitmap is missing required reverse index")); goto cleanup; } @@ -2142,7 +2142,7 @@ uint32_t *create_bitmap_mapping(struct bitmap_index *bitmap_git, if (!bitmap_is_midx(bitmap_git)) load_reverse_index(r, bitmap_git); - else if (load_midx_revindex(bitmap_git->midx) < 0) + else if (load_midx_revindex(bitmap_git->midx)) BUG("rebuild_existing_bitmaps: missing required rev-cache " "extension"); diff --git a/pack-revindex.c b/pack-revindex.c index db282dac8d..1f51b712e8 100644 --- a/pack-revindex.c +++ b/pack-revindex.c @@ -7,6 +7,7 @@ #include "trace2.h" #include "config.h" #include "midx.h" +#include "csum-file.h" struct revindex_entry { off_t offset; @@ -213,7 +214,8 @@ static int load_revindex_from_disk(char *revindex_name, fd = git_open(revindex_name); if (fd < 0) { - ret = -1; + /* "No file" means return 1. */ + ret = 1; goto cleanup; } if (fstat(fd, &st)) { @@ -265,7 +267,7 @@ cleanup: return ret; } -static int load_pack_revindex_from_disk(struct packed_git *p) +int load_pack_revindex_from_disk(struct packed_git *p) { char *revindex_name; int ret; @@ -303,6 +305,43 @@ int load_pack_revindex(struct repository *r, struct packed_git *p) return -1; } +/* + * verify_pack_revindex verifies that the on-disk rev-index for the given + * pack-file is the same that would be created if written from scratch. + * + * A negative number is returned on error. + */ +int verify_pack_revindex(struct packed_git *p) +{ + int res = 0; + + /* Do not bother checking if not initialized. */ + if (!p->revindex_map || !p->revindex_data) + return res; + + if (!hashfile_checksum_valid((const unsigned char *)p->revindex_map, p->revindex_size)) { + error(_("invalid checksum")); + res = -1; + } + + /* This may fail due to a broken .idx. */ + if (create_pack_revindex_in_memory(p)) + return res; + + for (size_t i = 0; i < p->num_objects; i++) { + uint32_t nr = p->revindex[i].nr; + uint32_t rev_val = get_be32(p->revindex_data + i); + + if (nr != rev_val) { + error(_("invalid rev-index position at %"PRIu64": %"PRIu32" != %"PRIu32""), + (uint64_t)i, nr, rev_val); + res = -1; + } + } + + return res; +} + int load_midx_revindex(struct multi_pack_index *m) { struct strbuf revindex_name = STRBUF_INIT; diff --git a/pack-revindex.h b/pack-revindex.h index 46e834064e..6dd47efea1 100644 --- a/pack-revindex.h +++ b/pack-revindex.h @@ -52,6 +52,22 @@ struct repository; int load_pack_revindex(struct repository *r, struct packed_git *p); /* + * Specifically load a pack revindex from disk. + * + * Returns 0 on success, 1 on "no .rev file", and -1 when there is an + * error parsing the .rev file. + */ +int load_pack_revindex_from_disk(struct packed_git *p); + +/* + * verify_pack_revindex verifies that the on-disk rev-index for the given + * pack-file is the same that would be created if written from scratch. + * + * A negative number is returned on error. + */ +int verify_pack_revindex(struct packed_git *p); + +/* * load_midx_revindex loads the '.rev' file corresponding to the given * multi-pack index by mmap-ing it and assigning pointers in the * multi_pack_index to point at it. diff --git a/t/t5325-reverse-index.sh b/t/t5325-reverse-index.sh index 0548fce1aa..431a603ca0 100755 --- a/t/t5325-reverse-index.sh +++ b/t/t5325-reverse-index.sh @@ -131,4 +131,78 @@ test_expect_success 'revindex in-memory vs on-disk' ' test_cmp on-disk in-core ) ' + +test_expect_success 'fsck succeeds on good rev-index' ' + test_when_finished rm -fr repo && + git init repo && + ( + cd repo && + + test_commit commit && + git -c pack.writeReverseIndex=true repack -ad && + git fsck 2>err && + test_must_be_empty err + ) +' + +test_expect_success 'set up rev-index corruption tests' ' + git init corrupt && + ( + cd corrupt && + + test_commit commit && + git -c pack.writeReverseIndex=true repack -ad && + + revfile=$(ls .git/objects/pack/pack-*.rev) && + chmod a+w $revfile && + cp $revfile $revfile.bak + ) +' + +corrupt_rev_and_verify () { + ( + pos="$1" && + value="$2" && + error="$3" && + + cd corrupt && + revfile=$(ls .git/objects/pack/pack-*.rev) && + + # Reset to original rev-file. + cp $revfile.bak $revfile && + + printf "$value" | dd of=$revfile bs=1 seek="$pos" conv=notrunc && + test_must_fail git fsck 2>err && + grep "$error" err + ) +} + +test_expect_success 'fsck catches invalid checksum' ' + revfile=$(ls corrupt/.git/objects/pack/pack-*.rev) && + orig_size=$(wc -c <$revfile) && + hashpos=$((orig_size - 10)) && + corrupt_rev_and_verify $hashpos bogus \ + "invalid checksum" +' + +test_expect_success 'fsck catches invalid row position' ' + corrupt_rev_and_verify 14 "\07" \ + "invalid rev-index position" +' + +test_expect_success 'fsck catches invalid header: magic number' ' + corrupt_rev_and_verify 1 "\07" \ + "reverse-index file .* has unknown signature" +' + +test_expect_success 'fsck catches invalid header: version' ' + corrupt_rev_and_verify 7 "\02" \ + "reverse-index file .* has unsupported version" +' + +test_expect_success 'fsck catches invalid header: hash function' ' + corrupt_rev_and_verify 11 "\03" \ + "reverse-index file .* has unsupported hash id" +' + test_done |