When we are processing a thin pack and making it non-thin we need
to update the header with a new object count. That causes us to
recompute the footer checksum for the entire pack, and the only
way to do that is to re-read the data from disk.
If there was filesystem corruption in the process (e.g. a bad
disk sector, or a kernel bug) we don't want to produce a valid
pack at the end. Instead we need to fail-fast with the error
so the user is aware of the corruption.
We now keep track of where the end of the original data is and
run two SHA-1 computations during the header-footer fixup. If
the original data region doesn't match the original footer we
got over the network we know there was corruption and we just
cannot trust this pack file.
Signed-off-by: Shawn O. Pearce <spearce@xxxxxxxxxxx>
---
This was inspired by the data corruption thread started
by J. Bruce Fields. At some point in the thread Linus
pointed out the C Git index-pack isn't as safe as it can
be, and offered a strategy to fix it.
We weren't as safe as we could be either in jgit, even
though we do verify CRC codes when we re-read a delta
base from disk. Not all objects are used as a base and
thus we could miss some forms of corruption.
.../src/org/spearce/jgit/transport/IndexPack.java | 17 ++++++++++++++++-
1 files changed, 16 insertions(+), 1 deletions(-)
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
index 29d99db..db9268e 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/IndexPack.java
@@ -156,6 +156,9 @@ public static IndexPack create(final Repository db, final InputStream is)
private byte[] packcsum;
+ /** If {@link #fixThin} this is the last byte of the original checksum. */
+ private long originalEOF;
+
/**
* Create a new pack indexer utility.
*
@@ -398,8 +401,9 @@ private void resolveChildDeltas(final long pos, int type, byte[] data,
private void fixThinPack(final ProgressMonitor progress) throws IOException {
growEntries();
+ originalEOF = packOut.length() - 20;
final Deflater def = new Deflater(Deflater.DEFAULT_COMPRESSION, false);
- long end = packOut.length() - 20;
+ long end = originalEOF;
for (final ObjectId baseId : new ArrayList<ObjectId>(baseById.keySet())) {
final ObjectLoader ldr = repo.openObject(baseId);
if (ldr == null)
@@ -453,9 +457,13 @@ private void writeWhole(final Deflater def, final int typeCode,
}
private void fixHeaderFooter() throws IOException {
+ final MessageDigest origDigest = Constants.newMessageDigest();
+ long origRemaining = originalEOF - 12;
+
packOut.seek(0);
if (packOut.read(buf, 0, 12) != 12)
throw new IOException("Cannot re-read pack header to fix count");
+ origDigest.update(buf, 0, 12);
NB.encodeInt32(buf, 8, entryCount);
packOut.seek(0);
packOut.write(buf, 0, 12);
@@ -466,9 +474,16 @@ private void fixHeaderFooter() throws IOException {
final int n = packOut.read(buf);
if (n < 0)
break;
+ if (origRemaining > 0) {
+ origDigest.update(buf, 0, (int) Math.min(n, origRemaining));
+ origRemaining -= n;
+ }
packDigest.update(buf, 0, n);
}
+ if (!Arrays.equals(origDigest.digest(), packcsum))
+ throw new IOException("Pack corrupted while writing to filesystem");
+
packcsum = packDigest.digest();
packOut.write(packcsum);
}
--
1.6.0.174.gd789c
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
