git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 3/5] fsck: correctly compute checksums on idx files larger than 4GB

From
Jeff King <peff@peff.net>
Date
Nov 13, 2020, 05:07 UTC
Message-ID
<20201113050714.GC744691@coredump.intra.peff.net>
In-Reply-To
<20201113050631.GA744608@coredump.intra.peff.net>

When checking the trailing checksum hash of a .idx file, we pass the whole buffer (minus the trailing hash) into a single call to the_hash_algo->update_fn(). But we cast it to an "unsigned int". This comes from c4001d92be (Use off_t when we really mean a file offset., 2007-03-06). That commit started storing the index_size variable as an off_t, but our mozilla-sha1 implementation from the time was limited to a smaller size. Presumably the cast was a way of annotating that we expected .idx files to be small, and so we didn't need to loop (as we do for arbitrarily-large .pack files). Though as an aside it was still wrong, because the mozilla function actually took a signed int.

These days our hash-update functions are defined to take a size_t, so we can pass the whole buffer in directly. The cast is actually causing a buggy truncation!

While we're here, though, let's drop the confusing off_t variable in the first place. We're getting the size not from the filesystem anyway, but from p->index_size, which is a size_t. In fact, we can make the code a bit more readable by dropping our local variable duplicating p->index_size, and instead have one that stores the size of the actual index data, minus the trailing hash.

Signed-off-by: Jeff King <peff@peff.net>
---
 pack-check.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/pack-check.c b/pack-check.c
index db3adf8781..4b089fe8ec 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -164,22 +164,22 @@ static int verify_packfile(struct repository *r,
 
 int verify_pack_index(struct packed_git *p)
 {
-	off_t index_size;
+	size_t len;
 	const unsigned char *index_base;
 	git_hash_ctx ctx;
 	unsigned char hash[GIT_MAX_RAWSZ];
 	int err = 0;
 
 	if (open_pack_index(p))
 		return error("packfile %s index not opened", p->pack_name);
-	index_size = p->index_size;
 	index_base = p->index_data;
+	len = p->index_size - the_hash_algo->rawsz;
 
 	/* Verify SHA1 sum of the index file */
 	the_hash_algo->init_fn(&ctx);
-	the_hash_algo->update_fn(&ctx, index_base, (unsigned int)(index_size - the_hash_algo->rawsz));
+	the_hash_algo->update_fn(&ctx, index_base, len);
 	the_hash_algo->final_fn(hash, &ctx);
-	if (!hasheq(hash, index_base + index_size - the_hash_algo->rawsz))
+	if (!hasheq(hash, index_base + len))
 		err = error("Packfile index for %s hash mismatch",
 			    p->pack_name);
 	return err;
-- 
2.29.2.705.g306f91dc4e
Previous: Jeff KingNext: Jeff King
Message 4 of 19 in “handling 4GB .idx files”
  1. 0/5 handling 4GB .idx filesJeff King, Nov 13, 2020
  2. 1/5 compute pack .idx byte offsets using size_tJeff King, Nov 13, 2020
  3. 2/5 use size_t to store pack .idx byte offsetsJeff King, Nov 13, 2020
  4. 3/5 fsck: correctly compute checksums on idx files larger than 4GBJeff King, Nov 13, 2020
  5. 4/5 block-sha1: take a size_t length parameterJeff King, Nov 13, 2020
  6. 5/5 packfile: detect overflow in .idx file size checksJeff King, Nov 13, 2020
  7. Johannes SchindelinNov 13, 2020
  8. Thomas BraunNov 15, 2020
  9. Jeff KingNov 16, 2020
  10. Derrick StoleeNov 16, 2020
  11. Jeff KingNov 16, 2020
  12. Thomas BraunNov 30, 2020
  13. Jeff KingDec 1, 2020
  14. t7900's new expensive testJeff King, Dec 1, 2020
  15. Derrick StoleeDec 1, 2020
  16. t7900: speed up expensive testJeff King, Dec 2, 2020
  17. Derrick StoleeDec 3, 2020
  18. Taylor BlauDec 1, 2020
  19. Jeff KingDec 2, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.