'git add' will happily corrupt a git repo if it is run while files in
the working directory are being modified.  A blob is added to the index
with contents that do not match its SHA1 hash.  If the index is then
committed, the corrupt blob cannot be checked out (or is checked out
with incorrect contents, depending on which tool you use to try to get
the file out of git) in the future.

Surprisingly, it's possible to clone, fetch, push, pull, and sometimes
even gc the corrupted repo several times before anyone notices the
corruption.  If the affected commit is included in a merge with history
from other git users, the only way to fix it is to rebase (or come up
with a blob whose contents match the affected SHA1 hash somehow).

It is usually possible to retrieve data committed before the corruption
by simply checking out an earlier tree in the affected branch's history.

The following shell code demonstrates this problem.  It runs a thread
which continuously modifies a file, and another thread that does
'git commit -am' and 'git fsck' in a continuous loop until corruption
is detected.  This might take up to 20 seconds on a slow machine.

	#!/bin/sh
	set -e

	# Create an empty git repo in /tmp/git-test
	rm -fr /tmp/git-test
	mkdir /tmp/git-test
	cd /tmp/git-test
	git init

	# Create a file named foo and add it to the repo
	touch foo
	git add foo

	# Thread 1:  continuously modify foo:
	while echo -n .; do
		dd if=/dev/urandom of=foo count=1024 bs=1k conv=notrunc >/dev/null 2>&1
	done &

	# Thread 2:  loop until the repo is corrupted
	while git fsck; do
		# Note the implied 'git add' in 'commit -a'
		# It will do the same with explicit 'git add'
		git commit -a -m'Test'
	done

	# Kill thread 1, we don't need it any more
	kill $!

	# Success!  Well, sort of.
	echo Repository is corrupted.  Have a nice day.

I discovered this bug accidentally when I was using inotifywait (from
the inotify-tools package) to automatically commit snapshots of a working
directory triggered by write events.

I tested this with a number of kernel versions from 2.6.27 to 2.6.31.
All of them reproduced this problem.  I checked this because strace
shows 'git add' doing a mmap(..., MAP_PRIVATE, ...) of its input file,
so I was wondering if there might have been a recent change in mmap()
behavior in either git or the kernel.

git 1.5.6.5 has this problem too, but some of the error messages are
different, and the problem sometimes manifests itself as silent corruption
of other objects (e.g. if someone checks out a corrupt tree and then does
'git add -u' or 'git commit -a', they will include the corrupt data in
their commit).

Hi gitsters,

Zygo Blaxell reported through http://bugs.debian.org/569505 that ‘git
update-index’ has some issues when the files it is adding change under
its feet:

My thoughts:

 - Low-hanging fruit: it should be possible for update-index to check
   the stat information to see if the file has changed between when it
   first opens it and when it finishes.

 - Zygo reported suppress that ‘git gc’ didn’t notice the problem.
   Should ‘git gc’ imply a ‘git fsck --no-full’?

 - Recovering from this kind of mistake in early history is indeed
   hard.  Any tricks for doing this?  Maybe fast-export | fast-import
   can do something with this, or maybe replace + filter-branch once
   it learns to be a little smarter.

 - How do checkout-index and cat-file blob react to a blob whose
   contents do not reflect its object name?  Are they behaving
   appropriately?  I would want cat-file blob to be able to retrieve
   such a broken blob’s contents, checkout-index not so much.

I imagine there are other things to learn, too.  The report and
reproduction recipe follow.

Thoughts?
Jonathan

Package: git-core
Version: 1:1.6.6.1-1
Severity: important

'git add' will happily corrupt a git repo if it is run while files in
the working directory are being modified.  A blob is added to the index
with contents that do not match its SHA1 hash.  If the index is then
committed, the corrupt blob cannot be checked out (or is checked out
with incorrect contents, depending on which tool you use to try to get
the file out of git) in the future.

Surprisingly, it's possible to clone, fetch, push, pull, and sometimes
even gc the corrupted repo several times before anyone notices the
corruption.  If the affected commit is included in a merge with history
from other git users, the only way to fix it is to rebase (or come up
with a blob whose contents match the affected SHA1 hash somehow).

It is usually possible to retrieve data committed before the corruption
by simply checking out an earlier tree in the affected branch's history.

The following shell code demonstrates this problem.  It runs a thread
which continuously modifies a file, and another thread that does
'git commit -am' and 'git fsck' in a continuous loop until corruption
is detected.  This might take up to 20 seconds on a slow machine.

	#!/bin/sh
	set -e

	# Create an empty git repo in /tmp/git-test
	rm -fr /tmp/git-test
	mkdir /tmp/git-test
	cd /tmp/git-test
	git init

	# Create a file named foo and add it to the repo
	touch foo
	git add foo

	# Thread 1:  continuously modify foo:
	while echo -n .; do
		dd if=/dev/urandom of=foo count=1024 bs=1k conv=notrunc >/dev/null 2>&1
	done &

	# Thread 2:  loop until the repo is corrupted
	while git fsck; do
		# Note the implied 'git add' in 'commit -a'
		# It will do the same with explicit 'git add'
		git commit -a -m'Test'
	done

	# Kill thread 1, we don't need it any more
	kill $!

	# Success!  Well, sort of.
	echo Repository is corrupted.  Have a nice day.

I discovered this bug accidentally when I was using inotifywait (from
the inotify-tools package) to automatically commit snapshots of a working
directory triggered by write events.

I tested this with a number of kernel versions from 2.6.27 to 2.6.31.
All of them reproduced this problem.  I checked this because strace
shows 'git add' doing a mmap(..., MAP_PRIVATE, ...) of its input file,
so I was wondering if there might have been a recent change in mmap()
behavior in either git or the kernel.

git 1.5.6.5 has this problem too, but some of the error messages are
different, and the problem sometimes manifests itself as silent corruption
of other objects (e.g. if someone checks out a corrupt tree and then does
'git add -u' or 'git commit -a', they will include the corrupt data in
their commit).

tags 569505 + upstream
severity 569505 normal
forwarded 569505 http://thread.gmane.org/gmane.linux.debian.devel.bugs.general/671958/focus=139648
thanks

Hi again,

Zygo Blaxell wrote:

I think this is a somewhat esoteric thing to do, so downgrading severity.

Still, thanks for the interesting report, and I do hope some of the issues
you mentioned are fixed soon.

Regards,
Jonathan

I don't think this is a good idea.  stat() is very coarse-grained, and
provides accuracy of only a second on a lot of file systems where git
working directories might be found.  If you run the test script on an
ext3 filesystem on a modern machine the stat() data won't change at all
even though the file contents change completely many times.

What would be a good idea is to make sure that the code that copies a
file into the index and calculates its hash does both in a single pass
over the same input data.  That might require replacing a simple mmap()
of the input file with a read-hash-copy loop.

 From mmap(2): "it is unspecified whether changes made to the file after
the mmap() call are visible in the mapped region".

You may think that doing a dummy "*p = *p" every 4096 bytes `fixes' it
(because it causes copy-on-write for every page) but even that does not
work because you can get a SIGBUS if the file is truncated while you
have it mapped privately (e.g. by fopen ("file", "w") or open with O_TRUNC).

Testcase:

#include <sys/mman.h>
#include <fcntl.h>
#include <stdio.h>

int
main ()
{
   system ("echo foo > file.test");
   int f = open ("file.test", O_RDONLY);
   char *p = mmap (NULL, 4096, PROT_READ, MAP_PRIVATE, f, 0);
   close (f);
   printf ("%s", p);       // prints "foo\n"

   f = open ("file.test", O_RDWR | O_CREAT | O_TRUNC, 0666);
   write (f, "bar\n", 4);  // comment out and next printf SIGSEGVs
   printf ("%s", p);       // prints "bar\n"
}

This means that MAP_PRIVATE is utterly useless.

Paolo

Paolo Bonzini <bonzini@gnu.org> writes:

I do not think we ever used MAP_PRIVATE in order to protect outselves from
uncontrolled changes made by the outside world in the first place.  Back
when most of these mmap calls were written by Linus and myself, we weren't
interested in using MAP_PRIVATE, or any other trick for that matter, to
deal with the case where the user tells git to go index a file, and then
mucks with the file before git finishes and gives back control.

We do use mmap in read-write mode when reading from the index file, and we
use MAP_PRIVATE to protect the outside world from our writing into the
mapped memory.  As far as I know that is the only mmap for which
MAP_PRIVATE matters in the core git codebase.

Our calls to mmap() almost all have MAP_PRIVATE, even for read-only mmap,
but that is more or less from inertia, aka "an existing call to mmap is
with these options, I'll add another call imitating that".

clone 569505 -1
retitle -1 teach 'git gc' to run 'git fsck --no-full'
severity -1 wishlist
notforwarded -1
tags 569505 + fixed-upstream
thanks

Hi again,

It seems like a good time to revisit this bug.

Zygo Blaxell wrote:
objects, 2010-02-21) was merged to master and will probably be
included in 1.7.1.  Its strategy:

	To do so we compute the SHA1 of the data being deflated
	_after_ the deflate operation has consumed that data, and
	make sure it matches with the expected SHA1.  This way we
	can rely on the CRC32 checked by the inflate operation to
	provide a good indication that the data is still coherent
	with its SHA1 hash.  One pathological case we ignore is
	when the data is modified before (or during) deflate
	call, but changed back before it is hashed.

Sounds reasonable to me.  Maybe it would be possible to try it out?
I can make a .deb from master for this if you would like.

Do you remember more details?  Some commands might be worth changing
to be more permissive (to more easily recover from corruption) or
strict (to more easily catch it).

For push, I do not think it is worth the slowdown; better to spend the
time needed to be robust in hash-object.

On the other hand, it does not seem overly burdensome to me to make
‘git gc’ imply a ‘git fsck --no-full’.  That wouldn’t be needed to
catch on-disk corruption (the CRC32 already does that), but it could
be a good idea anyway.  Cloning the bug.

I assume you used rebase -f?  Clever.

If it does not fail,

 git filter-branch --tree-filter :

should accomplish the same thing without making the history linear,
though it does not scale well to deep histories.  It should be
possible to devise an appropriate index-filter for this, too, if that
is too slow.

Yes, depending on how the file was changing this could be easy or hard
to do.  /usr/share/doc/git-doc/howto/recover-corrupted-blob-object.txt
is about something like this.

Ah, so the commits happened precisely during the race window.

“Something like git that can handle snapshots” tends to be something
people want every once in a while.  Of course, git has some problems
for these uses:

 - racy add, as you noticed;
 - checkout is not atomic or close to atomic;
 - large files are not supported well (but there is some work going on
   to change this);
 - uncompressible files are not supported well;
 - file metadata is not tracked;
 - directories with many entries are not supported well;
 - rename detection works poorly with binary files;
 - no quick way to throw away old history.

If the tree to be tracked is small (like /etc), one can try to work
around these things and get away with it for a while.

Still, when so many of git’s features are the opposite of useful for
the goal, it is a wonder it is so tempting to try anyway.  My guess is
it is the UI for browsing and manipulating history.

There are plenty of backup tools that are great for capturing
snapshots, but few of them make it easy to do much with the snapshots
once they’re captured.  The nicest I’ve experienced created a magic
.snapshots directory under the mount point for the file system whose
history was being tracked, with subdirectories for each time a
snapshot was taken.  Not nearly as pleasant to use as ‘git log’.

Something to think about.

I think this is more of a “MAP_PRIVATE by default because that’s
easier on the kernel” kind of thing.

Cheers,
Jonathan

I think whether you get no object vs. corrupt object depends on whether
the object is loose or packed.

Different git versions behave slightly differently, though I can't get
a corrupt data case at all at the moment.

git checkout gives you a working tree where corrupt files are missing,
and an index where corrupt files are marked deleted.

git filter-branch aborts when it sees the corrupt data if you have a
tree-filter, but if you only have an index-filter it will ignore
corrupt objects unless you do something to force it to examine their
contents.

filter-branch index-filter won't help you if other objects have been
deltaified based on corrupt objects--at that point, recovery is very hard.
I've only seen that occur on pack files that were corrupted outside of
git, though, so it's not a Git problem.

git gc will notice the corruption if it's packing corrupt loose objects.
It fails to notice if it's not packing loose objects, e.g. because
the loose objects are not old enough.

push and fetch cases are covered by the receive.fsckObjects config
variable.  It defaults to off, but if it's turned on then it will catch
the corruption at push/fetch time.  Obviously this doesn't help if you
have a corrupt repo and you never push/fetch it anywhere, and it also
doesn't help you if you don't change the setting from the default.

I reset to one commit before the corruption, then manually extract the
surviving changes between the commit after the corruption and the next
commit that modifies the corrupted file.  After that usually all future
commits can be cherry-picked.  After that gets pushed, everyone *else*
working on the project has to rebase their changes.

The problem isn't speed--the problem is tree-filter's requirement to check
out the data.  It can't, because the data is corrupt.  filter-branch does
check in that case, and it should (otherwise a filesystem on unreliable
media could spray undetected junk into your repo).

It's usually hard when the file was in some transient state during the
SHA1 calculation.  ;)

Only Git seems to have that.  SVN and CVS didn't.  Or maybe they did,
but they lacked the internal integrity-checking mechanisms to detect it.

Not a problem in my use cases.  Checkouts are very rare, usually only
occurring after some disaster or other.

"Large" is relative to the size of the system doing the work.  15 years
ago, 1MB was a "large" file; today, 1MB is on the high end of "small."

Much better than CVS.

Usually not a problem.

ext3 doesn't support that use case well either.  Fortunately it rarely
comes up.

Still better than CVS or SVN.

I don't intend to throw away old history at all.  Some of my snapshot
repos go back more than 10 years (they have been translated from CVS to
Subversion before Git).

Compression, integrity checking, and replication are the big wins for me.
The compression advantage of Git vs. other tools is not trivial.  Git
outperforms Subversion by something like 200:1.

The UI is important too, after a fashion--I have a bunch of tools
that notify me when they detect changes in some arbitrary git repo,
and it's easy to add snapshot repos to that notification framework.
It's usually more convenient to add the repos to the framework than to
add the framework to the repos, since that usually requires adding a
pile of extra tools on small or isolated machines.  Also, doing it
that way means the notification framework holds a backup copy of
the snapshot repo.

clone 569505 -1
tags -1 =
retitle -1 read-tree (?): silently skips some corrupted objects
thanks

Zygo Blaxell wrote:

Not good.  Will investigate.

Seems sensible.  Sometimes getting the right history requires repeated
invocations of filter-branch, so the ideal thing is to find some way
to examine (compare) the whole history before and after, and the next
best thing is to explicitly run a fsck before.

I think conventional wisdom is that in that case the best thing is to
explode the pack with git unpack-objects -r and recover what you can.
If there is crucial data that that misses, one can use git verify-pack -v
as a starting point to examine and repair the corruption.

Right, this could be changed.  I haven’t decided whether I think it’s
worth it (probably it is).

Oh, sounds more painful.

I guess I was expecting it to be easier because the object data is all
there; it just has the wrong SHA-1.  That is not the case in other
corruption scenarios, so maybe it is silly to spend too much time
thinking about how to deal with it, but I think it’s worth trying
anyway (at least maybe to write a script for contrib/).

It just does checkout-index, clean, and update-index; the only obvious
difference from a checkout + (munge) + add I can see is the clean.

Ah, I guess this happens with e.g. text editor swapfiles?  Ick.

I suspect SVN just uses a CRC32 computed at the same time as the files
are compressed, which indeed would not have the same problem.
http://svnbook.red-bean.com/nightly/en/svn.ref.svnadmin.c.verify.html

CVS and RCS I have no clue about.

True enough --- if you can wait to checkout until nothing cares about
what’s happening with those files (e.g. a shutdown), there’s no
problem.

I had trouble tracking a small repository of audio files I was
working on because of this.

Sure, as far as version control systems go, git is a good back up
systems, but what about backup systems?

Sadly, I don’t even know enough to say what replicating snapshot-based
backup system is the standard of care so to speak.

I guess if the history gets unmanageably big, one can start a new repo
and graft them together when needed.

I think any good backup system should have these things.  Your other
reasons are more compelling.  An unstated reason --- that git, like
cvs and svn, is a tool developers already often know quite well how to
use --- is also probably important.