On Fri, Oct 18, 2013 at 4:00 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: > On Fri, Oct 18, 2013 at 8:55 PM, Duy Nguyen <pclouds@xxxxxxxxx> wrote: >> On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland <johan@xxxxxxxxxxx> wrote: >>> diff --git a/sha1_file.c b/sha1_file.c >>> index f80bbe4..00ffffe 100644 >>> --- a/sha1_file.c >>> +++ b/sha1_file.c >>> @@ -2857,7 +2857,9 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename) >>> /* Make sure the directory exists */ >>> memcpy(buffer, filename, dirlen); >>> buffer[dirlen-1] = 0; >>> - if (mkdir(buffer, 0777) || adjust_shared_perm(buffer)) >>> + if (mkdir(buffer, 0777) && errno != EEXIST) >>> + return -1; >>> + if (adjust_shared_perm(buffer)) >>> return -1; >> >> I was going to ask what if the created directory does not have >> permission 0777. But adjust_shared_perm follows immediately after, so >> we're good. > > And I sent too early. adjust_shared_perm() does rely on current dir's > permission. So if the creator does "mkdir(buffer, 0)", we still have a > bad permission in the end. And adjust_shared_perm() is only active > when the repository is configured to be shared. I'm not sure where you're going with this... Whatever happens in another process between the first call git_mkstemp_mode() and our mkdir() call, one of three things will happen in this code: (a) mkdir() succeeds, i.e. we won the race. This case is unchanged by my patch. (b) mkdir() fails with errno != EEXIST. This results in us returning -1 to our caller. This case is also unchanged by my patch. (c) mkdir() fails with EEXIST, i.e. we lost the race. We do the adjust_shared_perms() which might fail (-> abort) or succeed, and we then _retry_ the git_mkstemp_mode(). There is no case where the "whatever" that happened between the first git_mkstemp_mode() and mkdir() will have a different outcome (create_tmpfile() failing or suceeding) than if it had happened _before_ the first git_mkstemp_mode(). And it is not our responsibility to control what other/unrelated processes might end up doing with directories inside .git/objects/... What am I missing? ...Johan -- Johan Herland, <johan@xxxxxxxxxxx> www.herland.net -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html