'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