Re: [PATCH] sha1_file.c:create_tmpfile(): Fix race when creating loose object dirs

From: Duy Nguyen <pclouds@gmail.com>
Date: Oct 18, 2013, 14:00 UTC
Message-ID: <CACsJy8ChkKQJJfV1G6DUQgq1Pwy7Hki1qkD8V4FsLC0bN362PA@mail.gmail.com>
In-Reply-To: <CACsJy8C=kaa2OgdxOrefP+Ck_sSA00tHs9UqMXwkh9v4K3Mdvg@mail.gmail.com>

On Fri, Oct 18, 2013 at 8:55 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Show 18 quoted lines

> On Fri, Oct 18, 2013 at 8:17 PM, Johan Herland <johan@herland.net> 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.

-- 
Duy