Jim Meyering <jim <at> meyering.net> writes:
> > 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.
>
> There is no need for O_DIRECTORY in that function, since the only thing
> it does with the resulting file descriptor is to apply fdopendir, and
> fdopendir will fail with ENOTDIR when fd refers to a non-directory.
Yes, fdopendir will reject non-directories, but the point of using O_DIRECTORY
up front is to avoid blocking on a FIFO prior to reaching the fdopendir.
>
> > Do we want to obey the FIXME and make opendirat an independent module?
>
> Sure, if there is another user.
> Is there?
Not so far, so I won't bother.
> > O_DIRECTORY|O_NOFOLLOW on
> > a symlink must produce failure, although POSIX is ambiguous whether the
failure
> > will be ELOOP or ENOTDIR)
I stand corrected. The POSIX wording is that O_DIRECTORY fails if "path does
not name a directory", but elsewhere in POSIX, that same phrase applies to
symlinks that point to directories. In other words:
open("link-to-dir",O_RDONLY|O_DIRECTORY) -> passes
open("link-to-file",O_RDONLY|O_DIRECTORY) -> fails with ENOTDIR
open("link-to-dir",O_RDONLY|O_DIRECTORY|O_NOFOLLOW) -> fails with ELOOP
open("link-to-file",O_RDONLY|O_DIRECTORY|O_NOFOLLOW) -> fails with either ELOOP
or ENOTDIR
> You might as well add all four,
> O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW
> on the off-chance that O_DIRECTORY makes the earlier openat
> fail, and it fails with a more useful errno value than fdopendir.
So blindly adding O_NOFOLLOW for opendirat is wrong.
Here's the latest draft of my patch.
>From 0145db62dc0f4fc77ed98119e40d256d95aa21cb Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Tue, 1 Sep 2009 14:06:37 -0600
Subject: [PATCH] fts: make directory fds more robust
* lib/fts.c (O_DIRECTORY): Let <fcntl.h> take care of this.
(opendirat): Specify O_DIRECTORY, and add fallbacks for safety.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 6 ++++++
lib/fts.c | 7 ++-----
2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ee81a23..58decf8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
2009-09-02 Eric Blake <e...@byu.net>
+ fts: make directory fds more robust
+ * lib/fts.c (O_DIRECTORY): Let <fcntl.h> take care of this.
+ (opendirat): Specify O_DIRECTORY, and add fallbacks for safety.
+
+2009-09-02 Eric Blake <e...@byu.net>
+
backupfile, chdir-long, fts, savedir: make safer
* 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 7616c6f..ebf66fc 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -228,10 +228,6 @@ static void free_dir (FTS *fts) {}
# define SIZE_MAX ((size_t) -1)
#endif
-#ifndef O_DIRECTORY
-# define O_DIRECTORY 0
-#endif
-
#define ISDOT(a) (a[0] == '.' && (!a[1] || (a[1] == '.' && !a[2])))
#define STREQ(a, b) (strcmp ((a), (b)) == 0)
@@ -309,7 +305,8 @@ 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 | O_NOCTTY | O_NONBLOCK);
DIR *dirp;
if (new_fd < 0)
--
1.6.3.2
