Re: [PATCH 1/3] prune-packed: fix a possible buffer overflow

[Jeff King <peff@peff.net>]

On Wed, Dec 18, 2013 at 11:44:58PM +0100, Michael Haggerty wrote:

> [While doing so, I got sidetracked by the question: what happens if a
> prune process deletes the "objects/XX" directory just the same moment
> that another process is trying to write an object into that directory?
> I think the relevant function is sha1_file.c:create_tmpfile().  It looks
> like there is a nonzero but very small race window that could result in
> a spurious "unable to create temporary file" error, but even then I
> don't think there would be any corruption or anything.]

There's a race there, but I think it's hard to trigger.

Our strategy with object creation is to call open, recognize ENOENT,
mkdir, and then try again. If the rmdir happens before our call to open,
then we're fine. If open happens first, then the rmdir will fail.

But we don't loop on ENOENT. So if the rmdir happens in the middle,
after the mkdir but before we call open again, we'd fail, because we
don't treat ENOENT specially in the second call to open. That is
unlikely to happen, though, as prune would not be removing a directory
it did not just enter and clean up an object from (in which case we
would not have gotten the first ENOENT in the creator). I think you'd So
you'd have to have something creating and then pruning the directory in
the time between our open and mkdir. It would probably be more likely to
see it if you had two prunes running (the first one kills the directory,
creator notices and calls mkdir, then the second prune kills the
directory, too).

So it seems unlikely and the worst case is a temporary failure, not a
corruption. It's probably not worth caring too much about, but we could
solve it pretty easily by looping on ENOENT on creation.

On a similar note, I imagine that a simultaneous "branch foo/bar" and
"branch -d foo/baz" could race over the creation/deletion of
"refs/heads/foo", but I didn't look into it.

-Peff