Eric Blake <ebb9 <at> byu.net> writes: > Here's the latest draft of my patch.
While we're at it, I noticed via findutils that fts leaks fds into child processes. This plugs the fts leak (but completely fixing find also requires a patch to findutils). Hmm - POSIX states that fdopendir can, but not must, set the cloexec flag, as part of consuming the fd. Maybe the gnulib fdopendir module should guarantee that the cloexec flag is set as part of creating a directory stream, rendering the first of the three hunks to fts.c redundant? Or maybe keep the fdopendir module as-is, but create a new module fdopendir-gnu, which goes further and gives the additional GNU semantics that: fd is changed to cloexec, and dirfd gives the same fd back (requires tweaking rpl_opendir on mingw to open an fd up front, and for all other platforms lacking fdopendir it requires writing into the member variable read by dirfd). From: Eric Blake <e...@byu.net> Date: Wed, 2 Sep 2009 14:44:51 -0600 Subject: [PATCH] fts: avoid leaking fds * modules/fts (Depends-on): Add cloexec. * lib/fts.c (opendirat, diropen, fts_build): Set close-on-exec flag. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 5 +++++ lib/fts.c | 18 ++++++++++++++---- modules/fts | 1 + 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index e63e020..3933600 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2009-09-02 Eric Blake <e...@byu.net> + fts: avoid leaking fds + * modules/fts (Depends-on): Add cloexec. + * lib/fts.c (opendirat, diropen, fts_build): Set close-on-exec + flag. + 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. diff --git a/lib/fts.c b/lib/fts.c index ebf66fc..c05eb8b 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -71,6 +71,9 @@ static char sccsid[] = "@(#)fts.c 8.6 (Berkeley) 8/14/94"; # include "fcntl--.h" # include "dirent--.h" # include "unistd--.h" +/* FIXME - use fcntl(F_DUPFD_CLOEXEC)/openat(O_CLOEXEC) once they are + supported. */ +# include "cloexec.h" # include "same-inode.h" #endif @@ -311,6 +314,7 @@ opendirat (int fd, char const *dir) if (new_fd < 0) return NULL; + set_cloexec_flag (new_fd, true); dirp = fdopendir (new_fd); if (dirp == NULL) { @@ -362,9 +366,12 @@ diropen (FTS const *sp, char const *dir) int open_flags = (O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK | (ISSET (FTS_PHYSICAL) ? O_NOFOLLOW : 0)); - return (ISSET (FTS_CWDFD) - ? openat (sp->fts_cwd_fd, dir, open_flags) - : open (dir, open_flags)); + int fd = (ISSET (FTS_CWDFD) + ? openat (sp->fts_cwd_fd, dir, open_flags) + : open (dir, open_flags)); + if (0 <= fd) + set_cloexec_flag (fd, true); + return fd; } FTS * @@ -1305,7 +1312,10 @@ fts_build (register FTS *sp, int type) if (nlinks || type == BREAD) { int dir_fd = dirfd(dirp); if (ISSET(FTS_CWDFD) && 0 <= dir_fd) - dir_fd = dup (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; diff --git a/modules/fts b/modules/fts index f80a827..9509557 100644 --- a/modules/fts +++ b/modules/fts @@ -8,6 +8,7 @@ lib/fts-cycle.c m4/fts.m4 Depends-on: +cloexec cycle-check d-ino d-type -- 1.6.3.2