Re: Troubleshoot clone issue to NFS.

From: Duy Nguyen <pclouds@gmail.com>
Date: May 24, 2015, 09:00 UTC
Message-ID: <CACsJy8Bb1O7QZtiPWdzYwYgOdV0dLDgD3Xu_YaWNUbsuTqJB5g@mail.gmail.com>
In-Reply-To: <20150522071224.GA10734@peff.net>

On Fri, May 22, 2015 at 2:12 PM, Jeff King <peff@peff.net> wrote:

Show 19 quoted lines

> So I think there are two possibilities for improving this:
>
>   1. Find places where we expect the object will not exist (like the
>      collision_test check you pointed out) and use a
>      "has_sha1_file_fast" that accepts that it may very occasionally
>      erroneously return false. In this case it would mean potentially
>      skipping a collision check, but I think that is OK. That could have
>      security implications, but only if an attacker:
>
>        a. has broken sha1 to generate a colliding object
>
>        b. can manipulate the victim into repacking in a loop
>
>        c. can manipulate the victim into fetching (or receiving a push)
>           simultaneously with (b)
>
>      at which point they can try to race the repack procedure to add
>      their colliding object to the repository. It seems rather unlikely
>      (especially part a).

In case you want to back away from option 2 because it starts to leak raciness, which your old commit tried to fix in the first place. I think the only other place that tests for lots of non-existent loose objects is write_sha1_file (e.g. "tar -xf bigtarball.tar.gz; cd bigtarball; git init; git add ."). But the number of calls should be much smaller compared to index-pack and it does not use has_sha1_file, it uses check_and_freshen_file() instead.

There are other places where has_sha1_file() may return 0, but I think the number of calls is even smaller to bother (shallow.c, fetch-pack.c, apply.c, buik-checkin.c)

-- 
Duy