I audited fts, getcwd, and glob for the possibility of dirfd returning
-1, and propose the following patch to fix all the problems I saw.
* For fts, rewrite to avoid dirfd entirely. This is doable now that
the fdopendir replacement doesn't close its argument. The trick is
to never invoke opendir.
* For getcwd, do not call dirfd. A call is no longer needed now that
the fdopendir replacement doesn't close its argument.
* For glob, add a comment explaining why dirfd can't return -1 there.
I haven't installed this since I'd like Jim's opinion first. I have
tested the changes with coreutils; its "make check" passes on RHEL 5.
(Coreutils doesn't use glob, but the change to glob is comments only.)
fts, getcwd, glob: audit for dirfd returning -1
* lib/fts.c (opendir): Remove #define; no longer used.
(opendirat): New arg PDIR_FD. All callers changed.
(fts_build, _opendir2): Use new opendirat to avoid the need for
dirfd, or for checking whether dirfd returns a negative value.
Don't use opendir; always use openat followed by fdopendir.
* lib/getcwd.c (__getcwd): Don't reset fd; fdopendir no longer clobbers
it.
* lib/glob.c (link_exists_p): Add comment explaining why dirfd never
returns -1 here.
* modules/fts (Depends-on): Remove dirfd.
* modules/getcwd (Depends-on): Likewise.
diff --git a/lib/fts.c b/lib/fts.c
index 1daf69c..40f1dee 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -160,8 +160,6 @@ enum Fts_stat
# define fchdir __fchdir
# undef open
# define open __open
-# undef opendir
-# define opendir __opendir
# undef readdir
# define readdir __readdir
#else
@@ -290,7 +288,7 @@ fts_set_stat_required (FTSENT *p, bool required)
/* FIXME: if others need this function, move it into lib/openat.c */
static inline DIR *
internal_function
-opendirat (int fd, char const *dir, int extra_flags)
+opendirat (int fd, char const *dir, int extra_flags, int *pdir_fd)
{
int new_fd = openat (fd, dir,
(O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
@@ -307,6 +305,7 @@ opendirat (int fd, char const *dir, int extra_flags)
close (new_fd);
errno = saved_errno;
}
+ *pdir_fd = new_fd;
return dirp;
}
@@ -1222,6 +1221,7 @@ fts_build (register FTS *sp, int type)
bool nostat;
size_t len, maxlen, new_len;
char *cp;
+ int dir_fd;
/* Set current node pointer. */
cur = sp->fts_cur;
@@ -1237,13 +1237,14 @@ fts_build (register FTS *sp, int type)
oflag = DTF_HIDEW|DTF_NODUP|DTF_REWIND;
#else
# define __opendir2(file, flag) \
- ( ! ISSET(FTS_NOCHDIR) && ISSET(FTS_CWDFD) \
- ? opendirat(sp->fts_cwd_fd, file, \
- ((ISSET(FTS_PHYSICAL) \
- && ! (cur->fts_level == FTS_ROOTLEVEL \
- && ISSET(FTS_COMFOLLOW))) \
- ? O_NOFOLLOW : 0)) \
- : opendir(file))
+ opendirat((! ISSET(FTS_NOCHDIR) && ISSET(FTS_CWDFD) \
+ ? sp->fts_cwd_fd : AT_FDCWD), \
+ file, \
+ ((ISSET(FTS_PHYSICAL) \
+ && ! (ISSET(FTS_COMFOLLOW) \
+ && cur->fts_level == FTS_ROOTLEVEL)) \
+ ? O_NOFOLLOW : 0), \
+ &dir_fd)
#endif
if ((dirp = __opendir2(cur->fts_accpath, oflag)) == NULL) {
if (type == BREAD) {
@@ -1306,8 +1307,7 @@ fts_build (register FTS *sp, int type)
* checking FTS_NS on the returned nodes.
*/
if (nlinks || type == BREAD) {
- int dir_fd = dirfd(dirp);
- if (ISSET(FTS_CWDFD) && 0 <= dir_fd)
+ if (ISSET(FTS_CWDFD))
{
dir_fd = dup (dir_fd);
if (0 <= dir_fd)
diff --git a/lib/getcwd.c b/lib/getcwd.c
index 7d47072..26e47f7 100644
--- a/lib/getcwd.c
+++ b/lib/getcwd.c
@@ -247,8 +247,6 @@ __getcwd (char *buf, size_t size)
dirstream = fdopendir (fd);
if (dirstream == NULL)
goto lose;
- /* Reset fd. It may have been closed by fdopendir. */
- fd = dirfd (dirstream);
fd_needs_closing = false;
#else
dirstream = __opendir (dotlist);
diff --git a/lib/glob.c b/lib/glob.c
index 03b55df..5be8c32 100644
--- a/lib/glob.c
+++ b/lib/glob.c
@@ -1282,6 +1282,8 @@ link_exists_p (int dfd, const char *dir, size_t dirlen,
const char *fname,
return link_exists2_p (dir, dirlen, fname, pglob);
else
{
+ /* dfd cannot be -1 here, because dirfd never returns -1 on
+ glibc, or on hosts that have fstatat. */
struct_stat64 st64;
return __fxstatat64 (_STAT_VER, dfd, fname, &st64, 0) == 0;
}
diff --git a/modules/fts b/modules/fts
index ba8cd57..0414ea1 100644
--- a/modules/fts
+++ b/modules/fts
@@ -13,7 +13,6 @@ cycle-check
d-ino
d-type
dirent-safer
-dirfd
fchdir
fcntl-h
fcntl-safer
diff --git a/modules/getcwd b/modules/getcwd
index c1544f5..108f14b 100644
--- a/modules/getcwd
+++ b/modules/getcwd
@@ -10,7 +10,6 @@ m4/getcwd.m4
Depends-on:
mempcpy
d-ino
-dirfd
extensions
memmove
openat
