[Top][All Lists]
[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