Re: [bug] Reliably Reproducible Bad Packing of Objects

[Duy Nguyen <pclouds@gmail.com>]

On Sat, Jun 25, 2016 at 12:38:22AM +0200, Christoph Michelbach wrote:
> Hi,
> 
> when run on a 32 bit system (Linux system, don't know about other
> systems),
> 
> mkdir test && cd test && git init && touch someFile && git add someFile
> && git commit -m "Initial commit." && dd if=/dev/urandom
> of=bigBinaryFile bs=1MB count=4294 && git add bigBinaryFile && git
> commit -m "Introduced big biary file."
> 
> reliably produces this error message: "error: bad packed object CRC
> for"


I tried with a 32-bit git build on 64-bit system and did not see this
message. Maybe it's because the 64-bit system. Just too lazy to bring
up a 32-bit VM to process 4GB of data.

But I did look around a bit, and the problem is probably number
truncation. Could you try this patch?

-- 8< --
diff --git a/sha1_file.c b/sha1_file.c
index d5e1121..cb571ac 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2281,7 +2281,7 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset,
 
 		if (do_check_packed_object_crc && p->index_version > 1) {
 			struct revindex_entry *revidx = find_pack_revindex(p, obj_offset);
-			unsigned long len = revidx[1].offset - obj_offset;
+			off_t len = revidx[1].offset - obj_offset;
 			if (check_pack_crc(p, &w_curs, obj_offset, len, revidx->nr)) {
 				const unsigned char *sha1 =
 					nth_packed_object_sha1(p, revidx->nr);
-- 8< --


If your system has large file support (you must or things must have
failed badly from the beginning), then both offsets in that
subtraction expression are of type off_t, aka 64-bit. However "len"
would be 32-bit and truncation can happen. check_pack_crc() takes
off_t too, so it seems well prepared already (but I didn't test).

PS. There's a similar statement in pack-objects.c, but I don't think
you have done anything to trigger it. OK, 'git gc' may..
--
Duy