[PATCH v2 0/4] pack-check: fix verification of large objects
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Feb 23, 2026, 16:00 UTC
- Message-ID
- <20260223-pks-fsck-fix-v2-0-99a0714ea3bd@pks.im>
- In-Reply-To
- <20260223-pks-fsck-fix-v1-0-c29036832b6e@pks.im>
Hi,
this small patch series addresses the bug reported by brian in [1]. Thanks!
Changes in v2:
- Extend the test to verify that we actually find corrupted objects in
both packs, even in the case where a non-corrupt version exists in
another pack.
- Reinstate `mark_packed_object_bad()`.
- Fix error checking for `git_parse_ulong()`.
- Disambiguate error conditions in `parse_object_with_flags()`.
- Link to v1: https://lore.kernel.org/r/20260223-pks-fsck-fix-v1-0-c29036832b6e@pks.imPatrick
[1]: <20260222183710.2963424-1-sandals@crustytoothpaste.net>
---
Patrick Steinhardt (4):
t/helper: improve "genrandom" test helper
object-file: adapt `stream_object_signature()` to take a stream
packfile: expose function to read object stream for an offset
pack-check: fix verification of large objectsobject-file.c | 10 +++------ object-file.h | 4 +++- object.c | 19 ++++++++++++++--- pack-check.c | 12 ++++++++--- packfile.c | 40 +++++++++++++++++++++-------------- packfile.h | 5 +++++ t/helper/test-genrandom.c | 5 ++++- t/t1006-cat-file.sh | 2 +- t/t1050-large.sh | 6 +++--- t/t1450-fsck.sh | 40 ++++++++++++++++++++++++++++++++++- t/t5301-sliding-window.sh | 2 +- t/t5310-pack-bitmaps.sh | 2 +- t/t5710-promisor-remote-capability.sh | 4 ++-- t/t7700-repack.sh | 6 +++--- 14 files changed, 114 insertions(+), 43 deletions(-)
Range-diff versus v1:
1: 1b1283e837 ! 1: daf895aef6 t/helper: improve "genrandom" test helper
@@ Commit message
have to precompute them.
Fix both of these issues by using `git_parse_ulong()` to parse the
- argumemnt. This function has better error handling, and it knows to
+ argument. This function has better error handling, and it knows to
handle unit suffixes.
Adapt a couple of our tests to use suffixes instead of manual
@@ t/helper/test-genrandom.c: int cmd__genrandom(int argc, const char **argv)
- count = (argc == 3) ? strtoul(argv[2], NULL, 0) : ULONG_MAX;
+ count = ULONG_MAX;
-+ if (argc == 3 && git_parse_ulong(argv[2], &count) < 0)
++ if (argc == 3 && !git_parse_ulong(argv[2], &count))
+ return error_errno("cannot parse argument '%s'", argv[2]);
while (count--) {
2: 9f25ed1a4b ! 2: ebca9efaec object-file: adapt `stream_object_signature()` to take a stream
@@ Commit message
a preconstructed stream. Prepare for this by accepting a stream as input
that the caller needs to assemble.
+ While at it, improve the error reporting in `parse_object_with_flags()`
+ to tell apart the two failure modes.
+
+ Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
## object-file.c ##
@@ object.c: struct object *parse_object_with_flags(struct repository *r,
- return NULL;
+ if (!skip_hash) {
+ struct odb_read_stream *stream = odb_read_stream_open(r->objects, oid, NULL);
-+ if (!stream || stream_object_signature(r, stream, repl) < 0) {
-+ error(_("hash mismatch %s"), oid_to_hex(oid));
-+ if (stream)
-+ odb_read_stream_close(stream);
++
++ if (!stream) {
++ error(_("unable to open object stream for %s"), oid_to_hex(oid));
+ return NULL;
+ }
+
-+ if (stream)
++ if (stream_object_signature(r, stream, repl) < 0) {
++ error(_("hash mismatch %s"), oid_to_hex(oid));
+ odb_read_stream_close(stream);
++ return NULL;
++ }
++
++ odb_read_stream_close(stream);
}
parse_blob_buffer(lookup_blob(r, oid));
return lookup_object(r, oid);
3: 9a28867564 ! 3: bead96797e packfile: expose function to read object stream for an offset
@@ packfile.c: static int close_istream_pack_non_delta(struct odb_read_stream *_st)
- struct packfile_store *store,
- const struct object_id *oid)
+int packfile_read_object_stream(struct odb_read_stream **out,
++ const struct object_id *oid,
+ struct packed_git *pack,
+ off_t offset)
{
@@ packfile.c: static int close_istream_pack_non_delta(struct odb_read_stream *_st)
switch (in_pack_type) {
default:
return -1; /* we do not do deltas for now */
++ case OBJ_BAD:
++ mark_bad_packed_object(pack, oid);
++ return -1;
+ case OBJ_COMMIT:
+ case OBJ_TREE:
+ case OBJ_BLOB:
@@ packfile.c: int packfile_store_read_object_stream(struct odb_read_stream **out,
stream->base.type = in_pack_type;
stream->base.size = size;
@@ packfile.c: int packfile_store_read_object_stream(struct odb_read_stream **out,
+ if (!find_pack_entry(store, oid, &e))
+ return -1;
+
-+ return packfile_read_object_stream(out, e.p, e.offset);
++ return packfile_read_object_stream(out, oid, e.p, e.offset);
+}
## packfile.h ##
@@ packfile.h: off_t get_delta_base(struct packed_git *p, struct pack_window **w_cu
off_t delta_obj_offset);
+int packfile_read_object_stream(struct odb_read_stream **out,
++ const struct object_id *oid,
+ struct packed_git *pack,
+ off_t offset);
+
4: 4eaf958e57 ! 4: 6b69624d81 pack-check: fix verification of large objects
@@ pack-check.c: static int verify_packfile(struct repository *r,
oid_to_hex(&oid), p->pack_name);
else if (!data &&
- (!(stream = odb_read_stream_open(r->objects, &oid, NULL)) ||
-+ (packfile_read_object_stream(&stream, p, entries[i].offset) < 0 ||
++ (packfile_read_object_stream(&stream, &oid, p, entries[i].offset) < 0 ||
stream_object_signature(r, stream, &oid) < 0))
err = error("packed %s from %s is corrupt",
oid_to_hex(&oid), p->pack_name);
@@ t/t1450-fsck.sh: test_expect_success 'fsck errors in packed objects' '
+ git init repo &&
+ (
+ cd repo &&
++
++ # We construct two packfiles with two objects in common and one
++ # object not in common. The objects in common can then be
++ # corrupted in one of the packfiles, respectively. The other
++ # objects that are unique to the packs are merely used to not
++ # have both packs contain the same data.
+ blob_one=$(test-tool genrandom one 200k | git hash-object -t blob -w --stdin) &&
+ blob_two=$(test-tool genrandom two 200k | git hash-object -t blob -w --stdin) &&
-+ printf "%s\n" "$blob_one" | git pack-objects .git/objects/pack/pack &&
-+ printf "%s\n" "$blob_two" | git pack-objects .git/objects/pack/pack &&
-+ remove_object "$blob_one" &&
-+ remove_object "$blob_two" &&
-+ git -c core.bigFileThreshold=100k fsck
++ blob_three=$(test-tool genrandom three 200k | git hash-object -t blob -w --stdin) &&
++ blob_four=$(test-tool genrandom four 200k | git hash-object -t blob -w --stdin) &&
++ pack_one=$(printf "%s\n" "$blob_one" "$blob_two" "$blob_three" | git pack-objects .git/objects/pack/pack) &&
++ pack_two=$(printf "%s\n" "$blob_two" "$blob_three" "$blob_four" | git pack-objects .git/objects/pack/pack) &&
++ chmod a+w .git/objects/pack/pack-*.pack &&
++
++ # Corrupt blob two in the first pack.
++ git verify-pack -v .git/objects/pack/pack-$pack_one >objects &&
++ offset_one=$(sed <objects -n "s/^$blob_two .* \(.*\)$/\1/p") &&
++ printf "\0" | dd of=.git/objects/pack/pack-$pack_one.pack bs=1 conv=notrunc seek=$offset_one &&
++
++ # Corrupt blob three in the second pack.
++ git verify-pack -v .git/objects/pack/pack-$pack_two >objects &&
++ offset_two=$(sed <objects -n "s/^$blob_three .* \(.*\)$/\1/p") &&
++ printf "\0" | dd of=.git/objects/pack/pack-$pack_two.pack bs=1 conv=notrunc seek=$offset_two &&
++
++ # We now expect to see two failures for the corrupted objects,
++ # even though they exist in a non-corrupted form in the
++ # respective other pack.
++ test_must_fail git -c core.bigFileThreshold=100k fsck 2>err &&
++ test_grep "unknown object type 0 at offset $offset_one in .git/objects/pack/pack-$pack_one.pack" err &&
++ test_grep "unknown object type 0 at offset $offset_two in .git/objects/pack/pack-$pack_two.pack" err
+ )
+'
+--- base-commit: 7c02d39fc2ed2702223c7674f73150d9a7e61ba4 change-id: 20260223-pks-fsck-fix-aa8a18a223c8