bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: unsafe use of dirfd in fts.c, getcwd.c, glob.c?


From: Jim Meyering
Subject: Re: unsafe use of dirfd in fts.c, getcwd.c, glob.c?
Date: Mon, 13 Sep 2010 07:30:05 +0200

Jim Meyering wrote:

> Paul Eggert wrote:
>> 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.
>
> Hi Paul,
>
> Nor do I.
>
>> 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.
>
> I see that getcwd.c and glob.c do assume that.
> E.g., here's getcwd.c's sole use of dirfd:
>
>       fd = dirfd (dirstream);
>       ...
>             entry_status = fstatat (fd, d->d_name, &st, AT_SYMLINK_NOFOLLOW);
>
> but fts.c uses dirfd like this:
>
>                 int dir_fd = dirfd(dirp);
>                 if (ISSET(FTS_CWDFD) && 0 <= dir_fd)
>                   {
>                     dir_fd = dup (dir_fd);
>                     set_cloexec_flag (dir_fd, true);
>                   }
>                 if (dir_fd < 0 || fts_safe_changedir(sp, cur, dir_fd, NULL)) {
>                         if (nlinks && type == BREAD)
>                                 cur->fts_errno = errno;
>                         cur->fts_flags |= FTS_DONTCHDIR;
>                         descend = false;
>                         closedir(dirp);
>                         if (ISSET(FTS_CWDFD) && 0 <= dir_fd)
>                                 close (dir_fd);
>                         dirp = NULL;
>                 } else
>                         descend = true;
>
> and while that code may call set_cloexec_flag on a negative DIR_FD
> (upon failed "dup" call -- I'll fix that, mainly for consistency),
> I don't see how it assumes dirfd never returns -1.

I've just pushed this:

>From 40af2cd5c8a49d2eaaaf6c0cd54ce0198282f9fa Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 13 Sep 2010 07:29:18 +0200
Subject: [PATCH] fts: don't operate on an invalid file descriptor after failed 
dup

* lib/fts.c (fts_build): Don't call set_cloexec_flag on a
negative file descriptor.
---
 ChangeLog |    6 ++++++
 lib/fts.c |    3 ++-
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8e6c3c2..9910da2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2010-09-13  Jim Meyering  <address@hidden>
+
+       fts: don't operate on an invalid file descriptor after failed dup
+       * lib/fts.c (fts_build): Don't call set_cloexec_flag on a
+       negative file descriptor.
+
 2010-09-12  Paul Eggert  <address@hidden>

        savedir: add streamsavedir, deprecate fdsavedir
diff --git a/lib/fts.c b/lib/fts.c
index a308a8c..4b89ee7 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1305,7 +1305,8 @@ fts_build (register FTS *sp, int type)
                 if (ISSET(FTS_CWDFD) && 0 <= dir_fd)
                   {
                     dir_fd = dup (dir_fd);
-                    set_cloexec_flag (dir_fd, true);
+                    if (0 <= dir_fd)
+                      set_cloexec_flag (dir_fd, true);
                   }
                 if (dir_fd < 0 || fts_safe_changedir(sp, cur, dir_fd, NULL)) {
                         if (nlinks && type == BREAD)
--
1.7.3.rc1.215.g6997c



reply via email to

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