[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
unsafe use of dirfd in fts.c, getcwd.c, glob.c?
From: |
Paul Eggert |
Subject: |
unsafe use of dirfd in fts.c, getcwd.c, glob.c? |
Date: |
Sun, 12 Sep 2010 19:42:02 -0700 |
User-agent: |
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.12) Gecko/20100826 Thunderbird/3.0.7 |
I ran into a problem with dirfd when coding up recent changes to GNU
tar, to fix some race conditions when traversing directories. Can
dirfd really return -1 on real platforms, when its argument is a valid
open directory stream? POSIX allows that behavior, but I don't know
of any implementations that do it.
If dirfd can return -1, it seems to me that fts.c, getcwd.c, and
glob.c need to be fixed, as a cursory look at them suggests that they
assume that dirfd never returns -1 in this situation.
Below is pseudocode for what I had to do in GNU tar to work around the
possibility of dirfd returning -1. The "if (fd2 < 0) { ... }" code
can be removed if we know -1 isn't possible. That code is pretty
annoying, not to mention less-safe in the presence of race conditions,
and almost inherently untestable. I'd rather rip it out if it's not
needed.
fd1 = openat (parentfd, "dir", ...);
if (fd1 < 0)
fail ("cannot open dir");
if (fstat (fd1, &oldst) != 0)
fail ("cannot get status of dir");
dirp = fdopendir (fd1);
if (! dirp)
fail ("cannot create stream for dir");
[ ... invoke readdir on dirp until we get all the directory entries ... ]
fd2 = dirfd (dirp);
if (fd2 < 0)
{
/* The dirent.h implementation doesn't use file descriptors
for directory streams, so open the directory again. */
if (closedir (dirp) != 0)
fail ("failure when closing stream for dir");
dirp = NULL; // (so that we don't close it later)
fd2 = openat (parentfd, "dir", ...);
if (fd2 < 0)
fail ("reopen failed for dir");
else if (! (fstat (fd2, &newst) == 0
&& oldst.st_ino == newst.st_ino
&& oldst.st_dev == newst.st_dev))
{
close (fd2);
fail ("impostor for dir");
}
}
[ now, use openat (fd2, "entry", ...) to open the entries one by one ]
[ when done, clean up as follows: ]
if ((dirp ? closedir (dirp) : close (fd2)) != 0)
fail ("failure when closing dir");
Here's the patch to 'tar' that the above is pseudocode for.
http://git.savannah.gnu.org/cgit/tar.git/commit/?id=8da503cad6e883b30c05749149084d24319063b4
- unsafe use of dirfd in fts.c, getcwd.c, glob.c?,
Paul Eggert <=