bug-gnulib
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]