# "failed to read delta base object at..." 16 messages from 2008-08-25 to 2008-08-27. Participants: J. Bruce Fields, Nicolas Pitre, Linus Torvalds, Jason McMullan, Junio C Hamano. Thread: https://gitlist.dev/t/15196 ## J. Bruce Fields, 2008-08-25 16:46 Subject: "failed to read delta base object at..." Message-ID: <20080825164602.GA2213@fieldses.org> URL: https://gitlist.dev/e/20080825164602.GA2213%40fieldses.org ``` Today I got this: fatal: failed to read delta base object at 3025976 from /home/bfields/local/linux-2.6/.git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack This has happened once before recently, I believe with a pack that had just been created on a recent fetch. (If I remember correctly, this was soon after a failed suspend/resume cycle that might have interrupted an in-progress fetch; could that possible explain the error?) In that case I reset origin/master, deleted a tag or two, and fetched, and the problem seemed to be fixed. --b. ``` ## Nicolas Pitre, 2008-08-25 18:58 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808251445090.1624%40xanadu.home In-Reply-To: <20080825164602.GA2213@fieldses.org> ``` On Mon, 25 Aug 2008, J. Bruce Fields wrote: > Today I got this: > > fatal: failed to read delta base object at 3025976 from > /home/bfields/local/linux-2.6/.git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack > > This has happened once before recently, I believe with a pack that had > just been created on a recent fetch. (If I remember correctly, this was > soon after a failed suspend/resume cycle that might have interrupted an > in-progress fetch; could that possible explain the error?) In that case > I reset origin/master, deleted a tag or two, and fetched, and the > problem seemed to be fixed. The above error is indicative of a corrupted pack on disk. To confirm it you could use 'git verify-pack' with the given pack file. With a sufficiently recent git, you only need to copy over another pack containing the corrupted object, or the object itself in loose form, into your object store to "fix" it. As to the source of disk corruptions... that's up to you to find the cause amongst many (including a failed suspend). Nicolas ``` ## Linus Torvalds, 2008-08-25 19:01 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808251153210.3363%40nehalem.linux-foundation.org In-Reply-To: <20080825164602.GA2213@fieldses.org> ``` On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > Today I got this: > > fatal: failed to read delta base object at 3025976 from > /home/bfields/local/linux-2.6/.git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack This is almost certainly due to some corruption. Basically, the call to "cache_or_unpack_entry()" failed, which in turn is because 'unpack_entry()' will have failed. And since you didn't see any other error, that failure is almost certainly due to unpack_compressed_entry() having failed. We don't print out _why_ (which is a bit sad), but the only thing that unpack_compressed_entry() does is to just "inflate()" the data at that offset. So it probably got a zlib data error, or an adler32 crc failure. > This has happened once before recently, I believe with a pack that had > just been created on a recent fetch. (If I remember correctly, this was > soon after a failed suspend/resume cycle that might have interrupted an > in-progress fetch; could that possible explain the error?) In that case > I reset origin/master, deleted a tag or two, and fetched, and the > problem seemed to be fixed. An interrupted fetch shouldn't have caused this, it really should only happen if you have some actual filesystem data error. Something didn't get written back correctly, or the page cache isn't coherent (or it got corrupted by something else like a wild kernel pointer, of course). Linus ``` ## J. Bruce Fields, 2008-08-25 21:18 Subject: Re: "failed to read delta base object at..." Message-ID: <20080825211830.GH2213@fieldses.org> URL: https://gitlist.dev/e/20080825211830.GH2213%40fieldses.org In-Reply-To: ``` On Mon, Aug 25, 2008 at 02:58:05PM -0400, Nicolas Pitre wrote: > On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > > Today I got this: > > > > fatal: failed to read delta base object at 3025976 from > > /home/bfields/local/linux-2.6/.git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack > > > > This has happened once before recently, I believe with a pack that had > > just been created on a recent fetch. (If I remember correctly, this was > > soon after a failed suspend/resume cycle that might have interrupted an > > in-progress fetch; could that possible explain the error?) In that case > > I reset origin/master, deleted a tag or two, and fetched, and the > > problem seemed to be fixed. > > The above error is indicative of a corrupted pack on disk. To confirm > it you could use 'git verify-pack' with the given pack file. That gives: error: Packfile .git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack SHA1 mismatch with itself No surprise, I assume. > With a sufficiently recent git, you only need to copy over another pack > containing the corrupted object, or the object itself in loose form, > into your object store to "fix" it. Yeah, I just did a git repack -a -d in a known good repository and copied the resulting pack over. Seems OK. Thanks. --b. > As to the source of disk corruptions... that's up to you to find the > cause amongst many (including a failed suspend). ``` ## J. Bruce Fields, 2008-08-25 21:31 Subject: Re: "failed to read delta base object at..." Message-ID: <20080825213104.GI2213@fieldses.org> URL: https://gitlist.dev/e/20080825213104.GI2213%40fieldses.org In-Reply-To: ``` Thanks to you and Nicolas for the responses. On Mon, Aug 25, 2008 at 12:01:42PM -0700, Linus Torvalds wrote: > On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > > > Today I got this: > > > > fatal: failed to read delta base object at 3025976 from > > /home/bfields/local/linux-2.6/.git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack > > This is almost certainly due to some corruption. Basically, the call to > "cache_or_unpack_entry()" failed, which in turn is because > 'unpack_entry()' will have failed. > > And since you didn't see any other error, that failure is almost certainly > due to unpack_compressed_entry() having failed. We don't print out _why_ > (which is a bit sad), but the only thing that unpack_compressed_entry() > does is to just "inflate()" the data at that offset. > > So it probably got a zlib data error, or an adler32 crc failure. > > > This has happened once before recently, I believe with a pack that had > > just been created on a recent fetch. (If I remember correctly, this was > > soon after a failed suspend/resume cycle that might have interrupted an > > in-progress fetch; could that possible explain the error?) In that case > > I reset origin/master, deleted a tag or two, and fetched, and the > > problem seemed to be fixed. > > An interrupted fetch shouldn't have caused this, it really should only > happen if you have some actual filesystem data error. Something didn't get > written back correctly, or the page cache isn't coherent (or it got > corrupted by something else like a wild kernel pointer, of course). OK. I seem to recall these pack files are created with something like open write sync close rename ? This is just ext3 with data=writeback on a local laptop disk, ubuntu's 2.6.24-21-generic. Would it be any use trying to look more closely at the pack in connection for any hints? (But with my git repo back I'm happy enough to just forget this for now if there's not anything obvious to try.) --b. ``` ## Linus Torvalds, 2008-08-25 21:37 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808251435540.3363%40nehalem.linux-foundation.org In-Reply-To: <20080825213104.GI2213@fieldses.org> ``` On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > OK. I seem to recall these pack files are created with something like > > open > write > sync > close > rename > > ? Yes. We're trying to be _extremely_ safe and only do things that should work for everything. > This is just ext3 with data=writeback on a local laptop disk, > ubuntu's 2.6.24-21-generic. Would it be any use trying to look more > closely at the pack in connection for any hints? You still have the packfile that caused problems available somewhere? If so, absolutely yes. If you have the corrupt pack, please make it available. > (But with my git repo back I'm happy enough to just forget this for now > if there's not anything obvious to try.) With the actual corrupt pack, we can make a fairly intelligent guess about exactly what the corruption was. Was it a flipped bit, or what? So if you have it, please do send it over. Linus ``` ## J. Bruce Fields, 2008-08-25 22:13 Subject: Re: "failed to read delta base object at..." Message-ID: <20080825221321.GL2213@fieldses.org> URL: https://gitlist.dev/e/20080825221321.GL2213%40fieldses.org In-Reply-To: ``` On Mon, Aug 25, 2008 at 02:37:39PM -0700, Linus Torvalds wrote: > > > On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > > > OK. I seem to recall these pack files are created with something like > > > > open > > write > > sync > > close > > rename > > > > ? > > Yes. We're trying to be _extremely_ safe and only do things that should > work for everything. > > > This is just ext3 with data=writeback on a local laptop disk, > > ubuntu's 2.6.24-21-generic. Would it be any use trying to look more > > closely at the pack in connection for any hints? > > You still have the packfile that caused problems available somewhere? If > so, absolutely yes. If you have the corrupt pack, please make it > available. > > > (But with my git repo back I'm happy enough to just forget this for now > > if there's not anything obvious to try.) > > With the actual corrupt pack, we can make a fairly intelligent guess about > exactly what the corruption was. Was it a flipped bit, or what? So if you > have it, please do send it over. OK! It's in: http://www.citi.umich.edu/u/bfields/bad-pack/ I assume the .idx file isn't interesting, but it's there anyway in case. --b. ``` ## Linus Torvalds, 2008-08-25 23:59 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808251616240.3363%40nehalem.linux-foundation.org In-Reply-To: <20080825221321.GL2213@fieldses.org> ``` On Mon, 25 Aug 2008, J. Bruce Fields wrote: > > OK! It's in: > > http://www.citi.umich.edu/u/bfields/bad-pack/ > > I assume the .idx file isn't interesting, but it's there anyway in case. It makes things slightly easier. But yes, the file is corrupt. The most obvious corruption is simply the comparison that git-fsck does with the SHA1 that the pack-file itself contains: all pack-files have at the end the SHA1 of the contents of the pack-file, and git fsck says: error: .git/objects/pack/pack-f7261d96cf1161b1b0a1593f673a67d0f2469e9b.pack SHA1 checksum mismatch so it's definitely not a valid pack-file, and perhaps more importantly, since git calculates the SHA1 as it writes the file out, the data really doesn't match what git wrote, and it got corrupted at some point in between the generation of the data and the reading back. (It _could_ still be some git corruption where git itself corrupted it after it had done the SHA1 writing, but I doubt it). As to the exact error: zlib reports "invalid distance too far back" as the error message when unpacking, which doesn't really tell me much. It's just a common data error. But since I actually _have_ that corrupt object in my pack too, I can actually look at what it should be. It's object 784716eb7fc5735a3e6b6209223508984013819f (it's a blob encoding for arch/arm/mach-pxa/sleep.S from earlier this year), and in my pack-file it's at byte offset 48094431-48098049. So I can actually extract just those bytes from my pack, and compare them against the bytes in your corrupt pack. It's interesting, because the differences aren't all that big, but they aren't a single word either. Here's a "diff -u" of "od -t x1" output: --- good-object.hex 2008-08-25 16:33:41.000000000 -0700 +++ bad-object.hex 2008-08-25 16:33:50.000000000 -0700 @@ -113,16 +113,16 @@ 0003400 e1 b1 dc ef 45 fa b1 ec 0f a0 2c 72 1b 25 55 f7 0003420 5a d6 f7 5c 72 dd 8b b2 44 dd 25 e6 c6 33 09 98 0003440 03 9b d1 49 8e 30 e6 7a df 76 76 bb cc c9 fb 44 -0003460 a0 14 8d 7d cd 12 75 81 2b c4 d9 71 27 62 ae b5 -0003500 87 a9 4f 76 c5 90 82 04 2b 15 53 47 ad 97 d1 02 -0003520 98 14 df 45 7c 5f 93 b1 08 fd b6 bf e1 62 1b 00 -0003540 b5 f6 ee 6c 70 30 78 73 74 38 3c 7f f3 e3 e5 90 -0003560 39 7e 2e df e5 e1 c5 c1 29 62 02 fd 61 36 61 7d -0003600 c0 83 a0 4f 17 04 02 c6 ec 59 2e c1 7d 20 41 b1 -0003620 fd 51 56 d0 b1 74 b9 76 81 4c 47 63 f0 10 76 15 -0003640 db a7 0b b8 d1 6a f8 59 73 8c c4 e5 c9 ab 8b cb -0003660 37 32 5d 4a 91 c3 c1 c9 77 33 b4 2c b7 61 a0 37 -0003700 47 f7 9b 3d b6 a5 99 5a cf 49 ef e0 3f 54 08 f4 +0003460 a0 14 8d 7d 22 00 00 00 6b 57 fe ff 55 57 fe ff +0003500 aa 57 fe ff b0 00 0e 00 d9 66 22 00 00 00 00 e0 +0003520 19 9f fe ff ff ff d1 43 fe ff d1 43 fe ff 00 c0 +0003540 bf fe f0 33 69 bf b2 33 69 bf 00 00 00 10 01 fe +0003560 bf fe 66 01 00 00 01 1f 37 17 2e 59 fe ff 00 00 +0003600 00 00 c2 41 fe ff 00 00 01 1f 37 17 44 42 fe ff +0003620 00 01 fe ff 01 1f 37 07 d4 42 fe ff 02 01 a8 44 +0003640 00 f8 bf fe 6b 57 fe ff 55 57 fe ff 97 57 fe ff +0003660 00 f8 bf fe 04 00 fe ff ab 45 fe ff 02 00 fe ff +0003700 ae 41 49 50 aa 40 fe ff cf 49 ef e0 3f 54 08 f4 0003720 1e 2b 6f 59 e5 47 06 f4 80 a7 76 3a 38 a7 88 0a 0003740 30 72 a2 0a ed e1 b0 d5 6d 6f 9e ca 96 c7 28 d0 0003760 2c 19 b6 db 51 97 a9 f6 c3 31 8a 50 22 87 7a 97 and the difference literally seems to be just a block of less than 160 bytes. But it's certainly not a single-bit error, and it's also not a disk block or anything like that. Looking at the first line that differs, they are: -0003460 a0 14 8d 7d cd 12 75 81 2b c4 d9 71 27 62 ae b5 +0003460 a0 14 8d 7d 22 00 00 00 6b 57 fe ff 55 57 fe ff so the differences start at offset 03464. The last line is: -0003700 47 f7 9b 3d b6 a5 99 5a cf 49 ef e0 3f 54 08 f4 +0003700 ae 41 49 50 aa 40 fe ff cf 49 ef e0 3f 54 08 f4 so they re-join at 03708 (octal offsets because of 'od' behavior). But in between there seems to be nothing in common. So the corrupt data looks like 22 00 00 00 6b 57 fe ff 55 57 fe ff 0003500 aa 57 fe ff b0 00 0e 00 d9 66 22 00 00 00 00 e0 0003520 19 9f fe ff ff ff d1 43 fe ff d1 43 fe ff 00 c0 0003540 bf fe f0 33 69 bf b2 33 69 bf 00 00 00 10 01 fe 0003560 bf fe 66 01 00 00 01 1f 37 17 2e 59 fe ff 00 00 0003600 00 00 c2 41 fe ff 00 00 01 1f 37 17 44 42 fe ff 0003620 00 01 fe ff 01 1f 37 07 d4 42 fe ff 02 01 a8 44 0003640 00 f8 bf fe 6b 57 fe ff 55 57 fe ff 97 57 fe ff 0003660 00 f8 bf fe 04 00 fe ff ab 45 fe ff 02 00 fe ff 0003700 ae 41 49 50 aa 40 fe ff and I don't see what the patern is, except that there's a lot of "fe ff" in there. 148 bytes, no obvious source. It definitely does _not_ look like compressed input (it's too regular to be something zlib spits out), nor is it text. Nor is it valid x86 assembly, so it's not some code-segment data either. And as far as I can tell, that's the _only_ corruption in the whole file, but I didn't really double-check. Does anybody see a pattern? Linus ``` ## Jason McMullan, 2008-08-26 20:43 Subject: Re: "failed to read delta base object at..." Message-ID: <48B46B04.70102@gmail.com> URL: https://gitlist.dev/e/48B46B04.70102%40gmail.com In-Reply-To: ``` Linus Torvalds wrote: > So the corrupt data looks like > > [snip] > > And as far as I can tell, that's the _only_ corruption in the whole file, > but I didn't really double-check. > > Does anybody see a pattern? > Was this pack created on a journaled file system? Reiserfs? Ext3? If there's journal corruption in a commonly used filesystem, That Would Be Bad. I would suspect the Reiserfs 'file tail' behavour and journalling have something to do with it, only because I still use and like ReiserV3+LVM (dynamic grow and offline shrink baby!). Only something like silently knifing files in the back would cause me to leave my beloved RedrumFS. Which I probably should. Eventually. Once ZFS changes it's license. HA! - Jason McMullan ``` ## J. Bruce Fields, 2008-08-26 20:55 Subject: Re: "failed to read delta base object at..." Message-ID: <20080826205552.GR4380@fieldses.org> URL: https://gitlist.dev/e/20080826205552.GR4380%40fieldses.org In-Reply-To: ``` On Mon, Aug 25, 2008 at 04:59:26PM -0700, Linus Torvalds wrote: > and the difference literally seems to be just a block of less than 160 > bytes. But it's certainly not a single-bit error, and it's also not a disk > block or anything like that. > > Looking at the first line that differs, they are: > > -0003460 a0 14 8d 7d cd 12 75 81 2b c4 d9 71 27 62 ae b5 > +0003460 a0 14 8d 7d 22 00 00 00 6b 57 fe ff 55 57 fe ff > > so the differences start at offset 03464. The last line is: > > -0003700 47 f7 9b 3d b6 a5 99 5a cf 49 ef e0 3f 54 08 f4 > +0003700 ae 41 49 50 aa 40 fe ff cf 49 ef e0 3f 54 08 f4 > > so they re-join at 03708 (octal offsets because of 'od' behavior). But in > between there seems to be nothing in common. > > So the corrupt data looks like > > 22 00 00 00 6b 57 fe ff 55 57 fe ff > 0003500 aa 57 fe ff b0 00 0e 00 d9 66 22 00 00 00 00 e0 > 0003520 19 9f fe ff ff ff d1 43 fe ff d1 43 fe ff 00 c0 > 0003540 bf fe f0 33 69 bf b2 33 69 bf 00 00 00 10 01 fe > 0003560 bf fe 66 01 00 00 01 1f 37 17 2e 59 fe ff 00 00 > 0003600 00 00 c2 41 fe ff 00 00 01 1f 37 17 44 42 fe ff > 0003620 00 01 fe ff 01 1f 37 07 d4 42 fe ff 02 01 a8 44 > 0003640 00 f8 bf fe 6b 57 fe ff 55 57 fe ff 97 57 fe ff > 0003660 00 f8 bf fe 04 00 fe ff ab 45 fe ff 02 00 fe ff > 0003700 ae 41 49 50 aa 40 fe ff > > and I don't see what the patern is, except that there's a lot of "fe ff" > in there. and all aligned on 2-byte boundaries? I don't see anything suggestive there either. > 148 bytes, no obvious source. It definitely does _not_ look like > compressed input (it's too regular to be something zlib spits out), nor is > it text. Nor is it valid x86 assembly, so it's not some code-segment data > either. > > And as far as I can tell, that's the _only_ corruption in the whole file, > but I didn't really double-check. > > Does anybody see a pattern? Got me. Thanks for taking a look at it. --b. ``` ## Jason McMullan, 2008-08-26 21:01 Subject: Re: "failed to read delta base object at..." Message-ID: <48B46F46.9090302@gmail.com> URL: https://gitlist.dev/e/48B46F46.9090302%40gmail.com In-Reply-To: <48B46B04.70102@gmail.com> ``` Jason McMullan wrote: > > Was this pack created on a journaled file system? Reiserfs? Ext3? > > If there's journal corruption in a commonly used filesystem, > That Would Be Bad. In private mail, it was indicated that this was an ext3 with the data=writeback option. From mount(1), man page, ext3 section: writeback Data ordering is not preserved - data may be written into the main file system after its metadata has been commit‐ ted to the journal. This is rumoured to be the highest- throughput option. It guarantees internal file system integrity, however it can allow old data to appear in files after a crash and journal recovery. All bets are off when data=writeback. - Jason McMullan ``` ## Linus Torvalds, 2008-08-27 17:05 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808270937340.3363%40nehalem.linux-foundation.org In-Reply-To: <48B46F46.9090302@gmail.com> ``` On Tue, 26 Aug 2008, Jason McMullan wrote: > > All bets are off when data=writeback. Not the way git writes pack-files. It does a fsync() before moving them into place (at least newer git versions do), so the data is stable. I do worry about wild pointers. I can't recognize the data, and it definitely doesn't look like any git internal data structures, but 16-bit data _is_ what zlib internally uses for things like the decoding tables. So if there is some use-after-free issue, I could imagine things like this happening inside of git. People do occasionally run valgrind on git, though, and it's been clean in the past, but I don't know if that has ever been done on the threaded packing, for example. For example, the corrupting data had patterns like this: 00 f8 bf fe 6b 57 fe ff 55 57 fe ff 97 57 fe ff where the pattern _could_ be something like { 00 f8 febf }, { 6b 57 fffe }, { 55 57 fffe }, { 97 57 fffe }, assuming that the "fe ff" pattern really is meaningful and is a 16-bit little-endian word. And the thign is, zlib "code" tables look exactly like that: typedef struct { unsigned char op; /* operation, extra bits, table bits */ unsigned char bits; /* bits in this part of the code */ unsigned short val; /* offset in table or code value */ } code; /* op values as set by inflate_table(): 00000000 - literal 0000tttt - table link, tttt != 0 is the number of table index bits 0001eeee - length or distance, eeee is the number of extra bits 01100000 - end of block 01000000 - invalid code */ but those particular op/val things don't make sense in that context either. But I don't know zlib that well, maybe the deflate routines use some other model. Linus ``` ## Nicolas Pitre, 2008-08-27 19:17 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808271458320.1624%40xanadu.home In-Reply-To: ``` On Wed, 27 Aug 2008, Linus Torvalds wrote: > > > On Tue, 26 Aug 2008, Jason McMullan wrote: > > > > All bets are off when data=writeback. > > Not the way git writes pack-files. It does a fsync() before moving them > into place (at least newer git versions do), so the data is stable. And isn't the bad data block size and alignment a bit odd for a filesystem crash corruption? > I do worry about wild pointers. I can't recognize the data, and it > definitely doesn't look like any git internal data structures, but 16-bit > data _is_ what zlib internally uses for things like the decoding tables. > > > So if there is some use-after-free issue, I could imagine things like this > happening inside of git. People do occasionally run valgrind on git, > though, and it's been clean in the past, but I don't know if that has ever > been done on the threaded packing, for example. However, in the pack-objects case, it is almost impossible to have such a corruption since the data is SHA1 summed immediately before being written out. In the index-pack case, it is even less likely since the data is SHA1 summed immediately after being written out. So there is no window for random pointer access corrupting the data without also influencing the pack checksum outcome. Therefore, if the pack content doesn't match its checksum, then the corruption must have occurred outside of git, otherwise the checksum would have included corrupted data already and the pack checksum would match. Nicolas ``` ## Linus Torvalds, 2008-08-27 19:48 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808271222250.3363%40nehalem.linux-foundation.org In-Reply-To: ``` On Wed, 27 Aug 2008, Nicolas Pitre wrote: > > And isn't the bad data block size and alignment a bit odd for a > filesystem crash corruption? Yes. If it was a filesystem issue, I'd expect it to be at least disk block aligned (512 bytes, most of the time) and more likely filesystem block aligned (ie mostly 4kB). However, if we were to re-write the file afterwards, it could still get non-block-aligned corruption - simply because there was a non-block-aligned rewrite that got lost. But we don't actually ever do that, except for the header and the SHA1 at the end in some unusual cases. > However, in the pack-objects case, it is almost impossible to have such > a corruption since the data is SHA1 summed immediately before being > written out. Yes. Anything that uses the "sha1write()" model (which includes the regular pack-file _and_ the index) should generally be pretty safe. However, we do have this odd case of fixing up the pack after-the-fact when we receive it from somebody else (because we get a thin pack and don't know how many objects the final result will have). And that case seems to be not as safe, because it - re-reads the file to recompute the SHA1 This is understandable, and it's fairly ok, but it does mean that there is a bigger chance of the SHA1 matching if something has corrupted the file in the meantime! (That was not the case of this corruption, obviously, since the SHA1 didn't match) - but it also forgets to fsync the result, because it only did that in one path rather in all cases of fixup. Again, this wasn't actually the cause of this corruption, because the corruption wasn't near the header or tail, so if it had been due to a missed write due to missing an fsync, the pattern would have been different. Anyway, we should fix the latter problem regardless, even if it's (a) damn unlikely and (b) definietly not the case in this thing. The fix is trivial - just move the "fsync_or_die()" into the fixup routine rather than doing it in one of the callers. Signed-off-by: Linus Torvalds Linus --- builtin-pack-objects.c | 1 - pack-write.c | 1 + 2 files changed, 1 insertions(+), 1 deletions(-) diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c index 2dadec1..d394c49 100644 --- a/builtin-pack-objects.c +++ b/builtin-pack-objects.c @@ -499,7 +499,6 @@ static void write_pack_file(void) } else { int fd = sha1close(f, NULL, 0); fixup_pack_header_footer(fd, sha1, pack_tmp_name, nr_written); - fsync_or_die(fd, pack_tmp_name); close(fd); } diff --git a/pack-write.c b/pack-write.c index a8f0269..ddcfd37 100644 --- a/pack-write.c +++ b/pack-write.c @@ -179,6 +179,7 @@ void fixup_pack_header_footer(int pack_fd, SHA1_Final(pack_file_sha1, &c); write_or_die(pack_fd, pack_file_sha1, 20); + fsync_or_die(pack_fd, pack_name); } char *index_pack_lockfile(int ip_out) ``` ## Junio C Hamano, 2008-08-27 20:14 Subject: Re: "failed to read delta base object at..." Message-ID: <7vy72inrlj.fsf@gitster.siamese.dyndns.org> URL: https://gitlist.dev/e/7vy72inrlj.fsf%40gitster.siamese.dyndns.org In-Reply-To: ``` Linus Torvalds writes: > And as far as I can tell, that's the _only_ corruption in the whole file, > but I didn't really double-check. Just FYI, replacing these 3619 bytes in JBF's packfile with the good object with dd conv=notrunc bs=1 seek=3025976 count=3619 makes the fixed pack pass verify-pack, so this part seems to be the only corruption. ``` ## Nicolas Pitre, 2008-08-27 20:46 Subject: Re: "failed to read delta base object at..." Message-ID: URL: https://gitlist.dev/e/alpine.LFD.1.10.0808271627540.1624%40xanadu.home In-Reply-To: ``` On Wed, 27 Aug 2008, Linus Torvalds wrote: > On Wed, 27 Aug 2008, Nicolas Pitre wrote: > > > However, in the pack-objects case, it is almost impossible to have such > > a corruption since the data is SHA1 summed immediately before being > > written out. > > Yes. Anything that uses the "sha1write()" model (which includes the > regular pack-file _and_ the index) should generally be pretty safe. What that means is that if git was the cause of the corruption itself then the pack would still match its checksum (verify-pâck would still fail nevertheless). > However, we do have this odd case of fixing up the pack after-the-fact > when we receive it from somebody else (because we get a thin pack and > don't know how many objects the final result will have). And that case > seems to be not as safe, because it > > - re-reads the file to recompute the SHA1 > > This is understandable, and it's fairly ok, but it does mean that there > is a bigger chance of the SHA1 matching if something has corrupted the > file in the meantime! I think that can be fixed. When reading the file back, it is possible to compute 2 sha1s: one to compare with the recieved one using original pack header, and the second which would be the final one. FRom a certain offset, new objects were added, so that first sha1 is validated against the received one and reset, and at the end, it should correspond to the sha1 of added objects that we should compute when writing them. Nicolas ```