From: "Shawn O. Pearce" <spearce@spearce.org>
To: Robin Rosenberg <robin.rosenberg@dewire.com>
Cc: git@vger.kernel.org
Subject: [JGIT PATCH 1/2] index-pack: Avoid disk corruption yielding a valid pack footer checksum
Date: Wed, 27 Aug 2008 14:48:43 -0700 [thread overview]
Message-ID: <1219873724-13348-1-git-send-email-spearce@spearce.org> (raw)
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@spearce.org>
---
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
next reply other threads:[~2008-08-27 21:49 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-08-27 21:48 Shawn O. Pearce [this message]
2008-08-27 21:48 ` [JGIT PATCH 2/2] index-pack: Use fsync to ensure received pack data is on disk Shawn O. Pearce
2008-08-28 0:09 ` [JGIT PATCH 1/2] index-pack: Avoid disk corruption yielding a valid pack footer checksum Nicolas Pitre
2008-08-28 0:14 ` Shawn O. Pearce
2008-08-28 0:26 ` Nicolas Pitre
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1219873724-13348-1-git-send-email-spearce@spearce.org \
--to=spearce@spearce.org \
--cc=git@vger.kernel.org \
--cc=robin.rosenberg@dewire.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).