2014-11-16 19:39 GMT+01:00 David Herrmann <[email protected]>: > Hi > > On Sun, Nov 16, 2014 at 7:34 PM, David Herrmann <[email protected]> wrote: >> Hi >> >> On Sun, Nov 9, 2014 at 3:42 PM, Ronny Chevalier >> <[email protected]> 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? Right
>> >> 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! It was the purpose of this patch. The stat() call was useless and we did not check if the mkdir() succeeded. But you are right, it does not fix any TOCTOU, I think I named it this way because of the coverity report. So you can change the commit message if you want :) > > Thanks > David _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
