[systemd-devel] [PATCH] udev: fix TOCTOU when creating a directory

[David Herrmann]

Hi

On Sun, Nov 16, 2014 at 7:34 PM, David Herrmann <dh.herrmann at gmail.com> wrote:
> Hi
>
> On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier
> <chevalier.ronny at gmail.com> wrote:
>> CID#979416
>> ---
>>  src/udev/collect/collect.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/udev/collect/collect.c b/src/udev/collect/collect.c
>> index dc849bd..6cb10fe 100644
>> --- a/src/udev/collect/collect.c
>> +++ b/src/udev/collect/collect.c
>> @@ -86,12 +86,13 @@ static void usage(void)
>>   */
>>  static int prepare(char *dir, char *filename)
>>  {
>> -        struct stat statbuf;
>>          char buf[512];
>>          int fd;
>> +        int r;
>>
>> -        if (stat(dir, &statbuf) < 0)
>> -                mkdir(dir, 0700);
>> +        r = mkdir(dir, 0700);
>> +        if (r < 0 && errno != EEXIST)
>> +                return -errno;
>>
>>          snprintf(buf, sizeof(buf), "%s/%s", dir, filename);
>
> So the race you describe is if the directory is removed after we
> stat() it, but before we use it somewhere down in the code. Applying
> your patch avoids the stat(), but it still fails if the dir is removed
> after your mkdir(). So this doesn't fix anything, does it?
>
> The code is definitely nicer than before, so I guess I'll apply it,
> anyway. But I don't see how it would fix anything, but silence a
> coverity warning. Or am I missing something? Feel free to prove me
> wrong ;)

One more addition: your code avoids an additional syscall, so yeah,
it's nicer. So I applied it now!

Thanks
David