Re: Problem with pack

From: Junio C Hamano <junkio@cox.net>
Date: Aug 27, 2006, 21:55 UTC
Message-ID: <7vpselx2qt.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <Pine.LNX.4.64.0608271102450.27779@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> writes:

> NOTE! This is all assuming my theory that a packed entry was broken in the 
> first place was correct. We obviously still don't _know_ what the problem 
> was. So far it's just a theory.

But it is a good theory. More plausible than alpha particle hitting the output buffer of zlib at the right moment, although the effects are the same ;-).

> So it might well be the case that we should simply add an extra integrity 
> check to the raw data copy in builtin-pack-objects.c: write_object().

I would agree that it is a sensible thing to do to insert check at places shown in the attached. The revalidate_pack_piece() would:

 - decode object header to make sure it decodes to sensible
   enum object_type value from the start of the buffer given as
   its first argument;

 - if it is of type OBJ_DELTA, skip 20-byte base object name;

 - the second argument is the length of the piece -- make sure
   the above steps did not require more than the length;

 - make sure the remainder is sane by running inflate() into
   void.  When fed the remainder in full, inflate() should
   return Z_OK.

For the first step we need to refactor unpack_object_header() in sha1_file.c a tiny bit to reuse it.

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 46f524d..0521cad 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -276,6 +282,7 @@ static unsigned long write_object(struct
 		map = map_sha1_file(entry->sha1, &mapsize);
 		if (map && !legacy_loose_object(map)) {
 			/* We can copy straight into the pack file */
+			revalidate_pack_piece(map, mapsize);
 			sha1write(f, map, mapsize);
 			munmap(map, mapsize);
 			written++;
@@ -319,6 +326,7 @@ static unsigned long write_object(struct
 
 		datalen = find_packed_object_size(p, entry->in_pack_offset);
 		buf = (char *) p->pack_base + entry->in_pack_offset;
+		revalidate_pack_piece(buf, datalen);
 		sha1write(f, buf, datalen);
 		unuse_packed_git(p);
 		hdrlen = 0; /* not really */