[PATCH] verify_packfile: check pack validity before accessing data

From: Jeff King <peff@peff.net>
Date: Sep 22, 2016, 03:49 UTC
Message-ID: <20160922034904.dm5okldfmgmin5d7@sigill.intra.peff.net>

The verify_packfile() does not explicitly open the packfile; instead, it starts with a sha1 checksum over the whole pack, and relies on use_pack() to open the packfile as a side effect.

If the pack cannot be opened for whatever reason (either because its header information is corrupted, or perhaps because a simultaneous repack deleted it), then use_pack() will die(), as it has no way to return an error. This is not ideal, as verify_packfile() otherwise tries to gently return an error (this lets programs like git-fsck go on to check other packs).

Instead, let's check is_pack_valid() up front, and return an error if it fails. This will open the pack as a side effect, and then use_pack() will later rely on our cached descriptor, and avoid calling die().

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-check.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/pack-check.c b/pack-check.c
index d123846..c5c7763 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -57,11 +57,8 @@ static int verify_packfile(struct packed_git *p,
 	int err = 0;
 	struct idx_entry *entries;
 
-	/* Note that the pack header checks are actually performed by
-	 * use_pack when it first opens the pack file.  If anything
-	 * goes wrong during those checks then the call will die out
-	 * immediately.
-	 */
+	if (!is_pack_valid(p))
+		return error("packfile %s cannot be accessed", p->pack_name);
 
 	git_SHA1_Init(&ctx);
 	do {

-- 
2.10.0.482.gae5a597