Re: fts: slightly more robust

[Eric Blake]

Jim Meyering <jim <at> meyering.net> writes:

> 
> I suspect that it'd be very hard to trigger these close failures,
> but it's better to be safe.

While we're visiting fts, how about this patch?  POSIX 2008 is clear that 
opendir should use O_DIRECTORY, so opendirat should probably do likewise.  Do 
we want to obey the FIXME and make opendirat an independent module?

Meanwhile, fts.c also has a diropen function which uses 
O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW.  Technically, the latter three are 
pointless given O_DIRECTORY (a directory is not a tty, is not a symlink, and 
does not block).  For two of the three, POSIX appears to allow the mix 
(O_NOCTTY is silently ignored; and the combination of O_DIRECTORY|O_NOFOLLOW on 
a symlink must produce failure, although POSIX is ambiguous whether the failure 
will be ELOOP or ENOTDIR).  But for the third, POSIX is explicit that the use 
of O_NONBLOCK on anything other than a fifo, block, or character device, the 
results are unspecified.

I can see why we are passing all the flags - if there is a kernel that supports 
some of the flags but not O_DIRECTORY, we have at least minimized other 
problems if we indeed attempt to open a non-directory.  But do we really want 
to be risking the unspecified behavior of O_NONBLOCK on a directory?  Or should 
we ask the Austin group to clarify that O_NONBLOCK is safely ignored on 
directories?  And should I be adding any (or all) of these three safety flags 
to opendirat?

I audited for other uses of fdopendir that might be lacking a useful 
O_DIRECTORY, but didn't find any.  In getcwd.c, the fd passed to fdopendir is 
always created via openat(fd,"..",O_RDONLY), and since ".." is already 
guaranteed to be a directory, the use of O_DIRECTORY would be redundant.  Any 
clients of savedir.c's fdsavedir are also candidates, but there weren't any 
within gnulib.


From: Eric Blake <address@hidden>
Date: Tue, 1 Sep 2009 14:06:37 -0600
Subject: [PATCH] fts: make directory fds more robust

* lib/fts.c (opendirat): Specify O_DIRECTORY.

Signed-off-by: Eric Blake <address@hidden>
---
 ChangeLog |    3 +++
 lib/fts.c |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a16fc6e..67c3359 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
 2009-09-01  Eric Blake  <address@hidden>

+       fts: make directory fds more robust
+       * lib/fts.c (opendirat): Specify O_DIRECTORY.
+
        dirent-safer: use it
        * lib/backupfile.c (includes): Use "dirent--.h", since
        numbered_backup can write to stderr during readdir.
diff --git a/lib/fts.c b/lib/fts.c
index 6373e44..6522d75 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -310,7 +310,7 @@ static inline DIR *
 internal_function
 opendirat (int fd, char const *dir)
 {
-  int new_fd = openat (fd, dir, O_RDONLY);
+  int new_fd = openat (fd, dir, O_RDONLY | O_DIRECTORY);
   DIR *dirp;

   if (new_fd < 0)
-- 
1.6.3.2