Eric Blake <ebb9 <at> byu.net> writes: > > I think we need to implement opendir_safer, alongside all the other > > *_safer modules. Otherwise, opendir can end up placing an open directory > > fd in one of the standard slots, and end up interfering with the intent of > > all the other *_safer wrappers. > > And here's at least one use case where it matters: > > $ find dir -mindepth 1 -ok echo {} \; <&- > < echo ... dir/sub > ? > $ echo $? > 0
Here's the preliminary patch series, to be applied on top of my fchdir/fdopendir series. However, since we are also lacking openat_safer, it looks like fts will STILL pollute the standard fds. I'll have to add in another patch for openat-safer, then test with findutils, before calling this series ready for prime-time. From: Eric Blake <e...@byu.net> Date: Tue, 1 Sep 2009 07:41:28 -0600 Subject: [PATCH 1/2] dirent-safer: new module * modules/dirent-safer: New file. * lib/dirent--.h: Likewise. * lib/dirent-safer.h: Likewise. * lib/opendir-safer.c: Likewise. * m4/dirent-safer.m4: Likewise. * MODULES.html.sh (Enhancements for POSIX:2008): Mention it. * modules/dirent-safer-tests: New test. * tests/test-dirent-safer.c: New file. * lib/fdopendir.c (includes): Ensure fdopendir is also safe. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 11 +++++ MODULES.html.sh | 1 + lib/dirent--.h | 23 ++++++++++ lib/dirent-safer.h | 22 +++++++++ lib/fdopendir.c | 4 ++ lib/opendir-safer.c | 68 ++++++++++++++++++++++++++++ m4/dirent-safer.m4 | 11 +++++ modules/dirent-safer | 28 ++++++++++++ modules/dirent-safer-tests | 11 +++++ tests/test-dirent-safer.c | 106 ++++++++++++++++++++++++++++++++++++++++++++ 10 files changed, 285 insertions(+), 0 deletions(-) create mode 100644 lib/dirent--.h create mode 100644 lib/dirent-safer.h create mode 100644 lib/opendir-safer.c create mode 100644 m4/dirent-safer.m4 create mode 100644 modules/dirent-safer create mode 100644 modules/dirent-safer-tests create mode 100644 tests/test-dirent-safer.c diff --git a/ChangeLog b/ChangeLog index ed1736a..44bd320 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2009-09-01 Eric Blake <e...@byu.net> + dirent-safer: new module + * modules/dirent-safer: New file. + * lib/dirent--.h: Likewise. + * lib/dirent-safer.h: Likewise. + * lib/opendir-safer.c: Likewise. + * m4/dirent-safer.m4: Likewise. + * MODULES.html.sh (Enhancements for POSIX:2008): Mention it. + * modules/dirent-safer-tests: New test. + * tests/test-dirent-safer.c: New file. + * lib/fdopendir.c (includes): Ensure fdopendir is also safe. + fdopendir: optimize on mingw * lib/unistd.in.h (_gl_directory_name): New prototype. * lib/fchdir.c (_gl_directory_name): Implement it. diff --git a/MODULES.html.sh b/MODULES.html.sh index 027a0bc..bb99642 100755 --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2394,6 +2394,7 @@ func_all_modules () func_begin_table func_module chdir-long + func_module dirent-safer func_module dirname func_module getopt func_module iconv_open-utf diff --git a/lib/dirent--.h b/lib/dirent--.h new file mode 100644 index 0000000..088aafd --- /dev/null +++ b/lib/dirent--.h @@ -0,0 +1,23 @@ +/* Like dirent.h, but redefine some names to avoid glitches. + + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Eric Blake. */ + +#include "dirent-safer.h" + +#undef opendir +#define opendir opendir_safer diff --git a/lib/dirent-safer.h b/lib/dirent-safer.h new file mode 100644 index 0000000..0debe26 --- /dev/null +++ b/lib/dirent-safer.h @@ -0,0 +1,22 @@ +/* Invoke dirent-like functions, but avoid some glitches. + + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Eric Blake. */ + +#include <dirent.h> + +DIR *opendir_safer (const char *name); diff --git a/lib/fdopendir.c b/lib/fdopendir.c index 3bc13ac..14dc111 100644 --- a/lib/fdopendir.c +++ b/lib/fdopendir.c @@ -27,6 +27,10 @@ #include "openat-priv.h" #include "save-cwd.h" +#if GNULIB_DIRENT_SAFER +# include "dirent--.h" +#endif + /* Replacement for Solaris' function by the same name. <http://www.google.com/search?q=fdopendir+site:docs.sun.com> First, try to simulate it via opendir ("/proc/self/fd/FD"). Failing diff --git a/lib/opendir-safer.c b/lib/opendir-safer.c new file mode 100644 index 0000000..8c399d0 --- /dev/null +++ b/lib/opendir-safer.c @@ -0,0 +1,68 @@ +/* Invoke opendir, but avoid some glitches. + + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Eric Blake. */ + +#include <config.h> + +#include "dirent-safer.h" + +#include <errno.h> +#include <unistd.h> +#include "unistd-safer.h" + +/* Like opendir, but do not clobber stdin, stdout, or stderr. */ + +DIR * +opendir_safer (char const *name) +{ + DIR *dp = opendir (name); + + if (dp) + { + int fd = dirfd (dp); + + if (0 <= fd && fd <= STDERR_FILENO) + { + /* If fdopendir is native (as on Linux), then it is safe to + assume dirfd(fdopendir(n))==n. If we are using the + gnulib module fdopendir, then this guarantee is not met, + but fdopendir recursively calls opendir_safer up to 3 + times to at least get a safe fd. If fdopendir is not + present but dirfd is accurate (as on cygwin 1.5.x), then + we recurse up to 3 times ourselves. Finally, if dirfd + always fails (as on mingw), then we are already safe. */ + DIR *newdp; + int e; +#if HAVE_FDOPENDIR || GNULIB_FDOPENDIR + int f = dup_safer (fd); + newdp = fdopendir (f); + e = errno; + if (! newdp) + close (f); +#else /* !FDOPENDIR */ + newdp = opendir_safer (name); + e = errno; +#endif + closedir (dp); + errno = e; + dp = newdp; + } + } + + return dp; +} diff --git a/m4/dirent-safer.m4 b/m4/dirent-safer.m4 new file mode 100644 index 0000000..6e8e6c4 --- /dev/null +++ b/m4/dirent-safer.m4 @@ -0,0 +1,11 @@ +#serial 1 +dnl Copyright (C) 2009 Free Software Foundation, Inc. +dnl This file is free software; the Free Software Foundation +dnl gives unlimited permission to copy and/or distribute it, +dnl with or without modifications, as long as this notice is preserved. + +AC_DEFUN([gl_DIRENT_SAFER], +[ + AC_CHECK_FUNCS_ONCE([fdopendir]) + AC_LIBOBJ([opendir-safer]) +]) diff --git a/modules/dirent-safer b/modules/dirent-safer new file mode 100644 index 0000000..47db18e --- /dev/null +++ b/modules/dirent-safer @@ -0,0 +1,28 @@ +Description: +Directory functions that avoid clobbering STD{IN,OUT,ERR}_FILENO. + +Files: +lib/dirent--.h +lib/dirent-safer.h +lib/opendir-safer.c +m4/dirent-safer.m4 + +Depends-on: +dirent +dirfd +unistd-safer + +configure.ac: +gl_DIRENT_SAFER +gl_MODULE_INDICATOR([dirent-safer]) + +Makefile.am: + +Include: +"dirent-safer.h" + +License: +GPL + +Maintainer: +Eric Blake diff --git a/modules/dirent-safer-tests b/modules/dirent-safer-tests new file mode 100644 index 0000000..da87778 --- /dev/null +++ b/modules/dirent-safer-tests @@ -0,0 +1,11 @@ +Files: +tests/test-dirent-safer.c + +Depends-on: +dup2 + +configure.ac: + +Makefile.am: +TESTS += test-dirent-safer +check_PROGRAMS += test-dirent-safer diff --git a/tests/test-dirent-safer.c b/tests/test-dirent-safer.c new file mode 100644 index 0000000..aeb8342 --- /dev/null +++ b/tests/test-dirent-safer.c @@ -0,0 +1,106 @@ +/* Test that directory streams leave standard fds alone. + Copyright (C) 2009 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see <http://www.gnu.org/licenses/>. */ + +/* Written by Eric Blake <e...@byu.net>, 2009. */ + +#include <config.h> + +#include "dirent--.h" + +#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#include "unistd-safer.h" + +/* This test intentionally closes stderr. So, we arrange to have fd 10 + (outside the range of interesting fd's during the test) set up to + duplicate the original stderr. */ + +#define BACKUP_STDERR_FILENO 10 +static FILE *myerr; + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (myerr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (myerr); \ + abort (); \ + } \ + } \ + while (0) + +int +main () +{ + int i; + DIR *dp; + /* The dirent-safer module works without the use of fdopendir (which + would also pull in fchdir and openat); but if those modules were + also used, we ensure that they are safe. In particular, the + gnulib version of fdopendir is unable to guarantee that + dirfd(fdopendir(fd))==fd, but we can at least guarantee that if + they are not equal, the fd returned by dirfd is safe. */ +#if HAVE_FDOPENDIR || GNULIB_FDOPENDIR + int dfd; +#endif + + /* We close fd 2 later, so save it in fd 10. */ + if (dup2 (STDERR_FILENO, BACKUP_STDERR_FILENO) != BACKUP_STDERR_FILENO + || (myerr = fdopen (BACKUP_STDERR_FILENO, "w")) == NULL) + return 2; + +#if HAVE_FDOPENDIR || GNULIB_FDOPENDIR + dfd = open (".", O_RDONLY); + ASSERT (STDERR_FILENO < dfd); +#endif + + /* Four iterations, with progressively more standard descriptors + closed. */ + for (i = -1; i <= STDERR_FILENO; i++) + { + if (0 <= i) + ASSERT (close (i) == 0); + dp = opendir ("."); + ASSERT (dp); + ASSERT (dirfd (dp) == -1 || STDERR_FILENO < dirfd (dp)); + ASSERT (closedir (dp) == 0); + +#if HAVE_FDOPENDIR || GNULIB_FDOPENDIR + { + int fd = dup_safer (dfd); + ASSERT (STDERR_FILENO < fd); + dp = fdopendir (fd); + ASSERT (dp); + ASSERT (dirfd (dp) == -1 || STDERR_FILENO < dirfd (dp)); + ASSERT (closedir (dp) == 0); + errno = 0; + ASSERT (close (fd) == -1); + ASSERT (errno == EBADF); + } +#endif + } + +#if HAVE_FDOPENDIR || GNULIB_FDOPENDIR + ASSERT (close (dfd) == 0); +#endif + + return 0; +} -- 1.6.3.2 >From fb12e447a0473913b4c47d804a024c15b1d85c9f Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 1 Sep 2009 12:25:01 -0600 Subject: [PATCH 2/2] dirent-safer: use it * lib/backupfile.c (includes): Use "dirent--.h", since numbered_backup can write to stderr during readdir. * lib/fts.c (includes) [!_LIBC]: Likewise. * lib/savedir.c (includes): Likewise. * lib/getcwd.c: Document why opendir_safer is unused. * lib/glob.c: Likewise. * lib/scandir.c: Likewise. * modules/backupfile (Depends-on): Add dirent-safer. * modules/fts (Depends-on): Likewise. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 11 +++++++++++ lib/backupfile.c | 9 ++------- lib/fts.c | 1 + lib/getcwd.c | 7 +++++-- lib/glob.c | 7 +++++-- lib/savedir.c | 7 +------ lib/scandir.c | 8 ++++++++ modules/backupfile | 1 + modules/fts | 1 + modules/savedir | 1 + 10 files changed, 36 insertions(+), 17 deletions(-) diff --git a/ChangeLog b/ChangeLog index 44bd320..8c7ca9a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2009-09-01 Eric Blake <e...@byu.net> + dirent-safer: use it + * lib/backupfile.c (includes): Use "dirent--.h", since + numbered_backup can write to stderr during readdir. + * lib/fts.c (includes) [!_LIBC]: Likewise. + * lib/savedir.c (includes): Likewise. + * lib/getcwd.c: Document why opendir_safer is unused. + * lib/glob.c: Likewise. + * lib/scandir.c: Likewise. + * modules/backupfile (Depends-on): Add dirent-safer. + * modules/fts (Depends-on): Likewise. + dirent-safer: new module * modules/dirent-safer: New file. * lib/dirent--.h: Likewise. diff --git a/lib/backupfile.c b/lib/backupfile.c index 1420edd..f6cf737 100644 --- a/lib/backupfile.c +++ b/lib/backupfile.c @@ -1,7 +1,7 @@ /* backupfile.c -- make Emacs style backup file names Copyright (C) 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, - 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006 Free Software + 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify @@ -37,7 +37,7 @@ #include <unistd.h> -#include <dirent.h> +#include "dirent--.h" #ifndef _D_EXACT_NAMLEN # define _D_EXACT_NAMLEN(dp) strlen ((dp)->d_name) #endif @@ -80,11 +80,6 @@ of `digit' even when the host does not conform to POSIX. */ #define ISDIGIT(c) ((unsigned int) (c) - '0' <= 9) -/* The results of opendir() in this file are not used with dirfd and fchdir, - therefore save some unnecessary work in fchdir.c. */ -#undef opendir -#undef closedir - /* The extension added to file names to produce a simple (as opposed to numbered) backup file name. */ char const *simple_backup_suffix = "~"; diff --git a/lib/fts.c b/lib/fts.c index a30e38a..6373e44 100644 --- a/lib/fts.c +++ b/lib/fts.c @@ -69,6 +69,7 @@ static char sccsid[] = "@(#)fts.c 8.6 (Berkeley) 8/14/94"; #if ! _LIBC # include "fcntl--.h" +# include "dirent--.h" # include "openat.h" # include "unistd--.h" # include "same-inode.h" diff --git a/lib/getcwd.c b/lib/getcwd.c index b9e57d3..72344ba 100644 --- a/lib/getcwd.c +++ b/lib/getcwd.c @@ -1,4 +1,4 @@ -/* Copyright (C) 1991-1999, 2004-2008 Free Software Foundation, Inc. +/* Copyright (C) 1991-1999, 2004-2009 Free Software Foundation, Inc. This file is part of the GNU C Library. This program is free software: you can redistribute it and/or modify @@ -103,7 +103,10 @@ #endif /* The results of opendir() in this file are not used with dirfd and fchdir, - therefore save some unnecessary recursion in fchdir.c. */ + and we do not leak fds to any single-threaded code that could use stdio, + therefore save some unnecessary recursion in fchdir.c and opendir_safer.c. + FIXME - if the kernel ever adds support for multi-thread safety for + avoiding standard fds, then we should use opendir_safer. */ #undef opendir #undef closedir diff --git a/lib/glob.c b/lib/glob.c index 40cc9b3..42cd39b 100644 --- a/lib/glob.c +++ b/lib/glob.c @@ -1,4 +1,4 @@ -/* Copyright (C) 1991-2002, 2003, 2004, 2005, 2006, 2007, 2008 +/* Copyright (C) 1991-2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc. This file is part of the GNU C Library. @@ -186,7 +186,10 @@ static const char *next_brace_sub (const char *begin, int flags) __THROW; #ifndef _LIBC /* The results of opendir() in this file are not used with dirfd and fchdir, - therefore save some unnecessary work in fchdir.c. */ + and we do not leak fds to any single-threaded code that could use stdio, + therefore save some unnecessary recursion in fchdir.c and opendir_safer.c. + FIXME - if the kernel ever adds support for multi-thread safety for + avoiding standard fds, then we should use opendir_safer. */ # undef opendir # undef closedir diff --git a/lib/savedir.c b/lib/savedir.c index 8400145..5e69d38 100644 --- a/lib/savedir.c +++ b/lib/savedir.c @@ -26,7 +26,7 @@ #include <errno.h> -#include <dirent.h> +#include "dirent--.h" #ifndef _D_EXACT_NAMLEN # define _D_EXACT_NAMLEN(dp) strlen ((dp)->d_name) #endif @@ -41,11 +41,6 @@ # define NAME_SIZE_DEFAULT 512 #endif -/* The results of opendir() in this file are not used with dirfd and fchdir, - therefore save some unnecessary work in fchdir.c. */ -#undef opendir -#undef closedir - /* Return a freshly allocated string containing the file names in directory DIRP, separated by '\0' characters; the end is marked by two '\0' characters in a row. diff --git a/lib/scandir.c b/lib/scandir.c index 8b34070..54a74d5 100644 --- a/lib/scandir.c +++ b/lib/scandir.c @@ -45,6 +45,14 @@ # define __opendir opendir # define __closedir closedir # define __set_errno(val) errno = (val) + +/* The results of opendir() in this file are not used with dirfd and fchdir, + and we do not leak fds to any single-threaded code that could use stdio, + therefore save some unnecessary recursion in fchdir.c and opendir_safer.c. + FIXME - if the kernel ever adds support for multi-thread safety for + avoiding standard fds, then we should use opendir_safer. */ +# undef opendir +# undef closedir #endif #ifndef SCANDIR_CANCEL diff --git a/modules/backupfile b/modules/backupfile index 3f8ccfd..aaf20f3 100644 --- a/modules/backupfile +++ b/modules/backupfile @@ -11,6 +11,7 @@ m4/backupfile.m4 Depends-on: argmatch d-ino +dirent-safer dirname memcmp stdbool diff --git a/modules/fts b/modules/fts index 38b2256..95313a0 100644 --- a/modules/fts +++ b/modules/fts @@ -11,6 +11,7 @@ Depends-on: cycle-check d-ino d-type +dirent-safer dirfd fchdir fcntl-h diff --git a/modules/savedir b/modules/savedir index 4171b80..6699095 100644 --- a/modules/savedir +++ b/modules/savedir @@ -7,6 +7,7 @@ lib/savedir.c m4/savedir.m4 Depends-on: +dirent-safer fdopendir xalloc -- 1.6.3.2