From: Alex Riesen Date: Tue, 20 Oct 2009 15:14:26 GMT Subject: Re: git hang with corrupted .pack Message-ID: <81b0412b0910200814v269e91fbkd7841308685e1c54@mail.gmail.com> In-Reply-To: <7vbpk985t1.fsf@alter.siamese.dyndns.org> On Thu, Oct 15, 2009 at 09:39, Junio C Hamano wrote: > Nicolas Pitre writes: > >> I confirm this test without the fix reproduces the infinite loop (and >> does stall the test suite). > > Thanks, both of you. I seem to have problems with this change (on Cygwin). Sometimes accessing an object in a pack fails in unpack_compressed_entry. When it happens, both avail_in and avail_out of the stream are 0, and the reported status is Z_BUF_ERROR. Output with the second attached patch: error: *** inflate error: 0x862380 size=1256, avail_in=0 (was 697), avail_out=0 (was 1256) error: *** unpack_compressed_entry failed error: failed to read object 3296766eb5531ef051ae392114de5d75556f5613 at offset 2620741 from .git/objects/pack/pack-996206790aaefbf4d34c86b3ff546bb924546b7c.pack fatal: object 3296766eb5531ef051ae392114de5d75556f5613 is corrupted I cannot reproduce the problem on a normal system (a 64bit, reasonably modern Linux in my case). An attempt to use an upgraded zlib on this cygwin system was not successful, there was an updated library, but a clean recompile didn't change anything. I worked the case around by allocating a bit more than uncompressed data need. In case someone else also sees the problem, below is how. The size of the overallocation is arbitrary. diff --git a/sha1_file.c b/sha1_file.c index 4cc8939..66c2519 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1585,11 +1585,11 @@ static void *unpack_compressed_entry(struct packed_git *p, z_stream stream; unsigned char *buffer, *in; - buffer = xmalloc(size + 1); - buffer[size] = 0; + buffer = xmalloc(size + 8); + memset(buffer + size, 0, 8); memset(&stream, 0, sizeof(stream)); stream.next_out = buffer; - stream.avail_out = size; + stream.avail_out = size + 8; git_inflate_init(&stream); do { -- 1.6.5.59.g000dd The problematic repo is a little big to post (it's my cygwin-git repo), I'll have to find hosting for it first. From 643fdf50e4343c3ce3a4b99183dba8d35a10c877 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Tue, 20 Oct 2009 16:42:26 +0200 Subject: [PATCH 1/2] Workaround inflate sometimes fails to unpack data When it happens, zlib stream's avail_in and avail_out are 0, the returned status is Z_BUF_ERROR. The values weren't 0 before calling inflate. error: *** inflate error: 0x862380 size=1256, avail_in=0 (was 697), avail_out=0 (was 1256) error: *** unpack_compressed_entry failed error: failed to read object 3296766eb5531ef051ae392114de5d75556f5613 at offset 2620741 from .git/objects/pack/pack-996206790aaefbf4d34c86b3ff546bb924546b7c.pack fatal: object 3296766eb5531ef051ae392114de5d75556f5613 is corrupted --- sha1_file.c | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 4cc8939..66c2519 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1585,11 +1585,11 @@ static void *unpack_compressed_entry(struct packed_git *p, z_stream stream; unsigned char *buffer, *in; - buffer = xmalloc(size + 1); - buffer[size] = 0; + buffer = xmalloc(size + 8); + memset(buffer + size, 0, 8); memset(&stream, 0, sizeof(stream)); stream.next_out = buffer; - stream.avail_out = size; + stream.avail_out = size + 8; git_inflate_init(&stream); do { -- 1.6.5.59.g000dd From fc5b47d953f171d7591349acad9a23d3ec683ce9 Mon Sep 17 00:00:00 2001 From: Alex Riesen Date: Tue, 20 Oct 2009 15:53:33 +0200 Subject: [PATCH 2/2] Debugging strange failure in zlibs inflate --- sha1_file.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/sha1_file.c b/sha1_file.c index 66c2519..1bf240e 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1594,10 +1594,24 @@ static void *unpack_compressed_entry(struct packed_git *p, git_inflate_init(&stream); do { in = use_pack(p, w_curs, curpos, &stream.avail_in); + unsigned prev_ain = stream.avail_in; + unsigned prev_aout = stream.avail_out; + if (!stream.avail_in) + error("*** avail_in=0, inflate will fail!"); + if (!stream.avail_out) + error("*** avail_out=0, inflate will fail!"); stream.next_in = in; st = git_inflate(&stream, Z_FINISH); if (st == Z_BUF_ERROR && (stream.avail_in || !stream.avail_out)) + { + error("*** inflate error: %p size=%lu, " + "avail_in=%d (was %u), " + "avail_out=%d (was %u)", + buffer, size, + stream.avail_in, prev_ain, + stream.avail_out, prev_aout); break; + } curpos += stream.next_in - in; } while (st == Z_OK || st == Z_BUF_ERROR); git_inflate_end(&stream); @@ -1654,6 +1668,8 @@ static void *cache_or_unpack_entry(struct packed_git *p, off_t base_offset, delta_base_cached -= ent->size; } else { ret = xmemdupz(ent->data, ent->size); + if (!ret) + fprintf(stderr, "*** no memory\n"); } *type = ent->type; *base_size = ent->size; @@ -1815,6 +1831,8 @@ void *unpack_entry(struct packed_git *p, off_t obj_offset, case OBJ_BLOB: case OBJ_TAG: data = unpack_compressed_entry(p, &w_curs, curpos, *sizep); + if (!data) + error("*** unpack_compressed_entry failed"); break; default: data = NULL; -- 1.6.5.59.g000dd