-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 I'm currently testing the following series. Any thoughts or review on this?
Eric Blake (5): cloexec: add dup_cloexec Rather than having dup_noinherit, specific to w32spawn.h, it seems better to have a version also ported to Unix. Eventually, when I get around to implementing a partial rpl_fcntl, this will allow F_DUPFD_CLOEXEC for systems that lack it. test-dup2: enhance test Make sure that dup2 of a cloexec fd creates an inheritable fd. unistd-safer: allow preservation of cloexec status via flag Create fd_safer_flag; the counterpart to fd_safer that will honor O_CLOEXEC when it is defined. For now, that's pretty much only newer Linux kernels and mingw, but other platforms will slowly add POSIX 2008 compliance, and we are making progress towards having gnulib be able to provide an O_CLOEXEC emulation. stdlib-safer: preserve cloexec flag for mkostemp[s] Fix a bug in mkostemp_safer, where it was not preserving O_CLOEXEC status. pipe2-safer: new module Add pipe2_safer, and use it in the pipe module. - -- Don't work too hard, make some time for fun as well! Eric Blake e...@byu.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (Cygwin) Comment: Public key at home.comcast.net/~ericblake/eblake.gpg Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAksbPyUACgkQ84KuGfSFAYDJqACbBI6FgK43WYFdAcd2gAYBlE2i /AIAn37qLx/KHULwJc555duqljnpqmIE =KLW4 -----END PGP SIGNATURE-----
>From d8ccc13ba3ba05654f9be09bba5492cc8da30c3b Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Fri, 4 Dec 2009 22:10:44 -0700 Subject: [PATCH 1/5] cloexec: add dup_cloexec This is needed to enforce correct semantics of mkostemp_safer. Meanwhile, it is one step closer to providing O_CLOEXEC support to open, as well as implementing portions of fcntl for mingw. * lib/cloexec.h (dup_cloexec): New prototype. Add copyright header and comments. * lib/cloexec.c (set_cloexec_flag): Add comments. (dup_cloexec): New function, with mingw implementation borrowed from... * lib/w32spawn.h (dup_noinherit): ...here. * modules/execute (Depends-on): Add cloexec. * modules/pipe (Depends-on): Likewise. * modules/cloexec (Depends-on): Add dup2. * modules/cloexec-tests (Files): New file. * tests/test-cloexec.c: Likewise. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 13 +++++ lib/cloexec.c | 111 ++++++++++++++++++++++++++++++++++++++++++-- lib/cloexec.h | 36 ++++++++++++++ lib/w32spawn.h | 28 ++---------- modules/cloexec | 1 + modules/cloexec-tests | 10 ++++ modules/execute | 1 + modules/pipe | 1 + tests/test-cloexec.c | 122 +++++++++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 293 insertions(+), 30 deletions(-) create mode 100644 modules/cloexec-tests create mode 100644 tests/test-cloexec.c diff --git a/ChangeLog b/ChangeLog index c3d6d73..f0c2abd 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2009-12-05 Eric Blake <e...@byu.net> + cloexec: add dup_cloexec + * lib/cloexec.h (dup_cloexec): New prototype. Add copyright + header and comments. + * lib/cloexec.c (set_cloexec_flag): Add comments. + (dup_cloexec): New function, with mingw implementation borrowed + from... + * lib/w32spawn.h (dup_noinherit): ...here. + * modules/execute (Depends-on): Add cloexec. + * modules/pipe (Depends-on): Likewise. + * modules/cloexec (Depends-on): Add dup2. + * modules/cloexec-tests (Files): New file. + * tests/test-cloexec.c: Likewise. + test-xalloc-die: fix test for mingw * modules/xalloc-die-tests (Files): Add tests/init.sh. * tests/test-xalloc-die.sh: Rewrite to use init.sh. Strip diff --git a/lib/cloexec.c b/lib/cloexec.c index 0fb227c..b862cc6 100644 --- a/lib/cloexec.c +++ b/lib/cloexec.c @@ -1,6 +1,7 @@ /* closexec.c - set or clear the close-on-exec descriptor flag - Copyright (C) 1991, 2004, 2005, 2006 Free Software Foundation, Inc. + Copyright (C) 1991, 2004, 2005, 2006, 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 @@ -21,12 +22,27 @@ #include "cloexec.h" -#include <unistd.h> +#include <errno.h> #include <fcntl.h> +#include <unistd.h> + +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ +/* Native Woe32 API. */ +# define WIN32_LEAN_AND_MEAN +# include <windows.h> +# include <io.h> +#endif + /* Set the `FD_CLOEXEC' flag of DESC if VALUE is true, or clear the flag if VALUE is false. - Return 0 on success, or -1 on error with `errno' set. */ + Return 0 on success, or -1 on error with `errno' set. + + Note that on MingW, this function does NOT protect DESC from being + inherited into spawned children. Instead, either use dup_cloexec + followed by closing the original DESC, or use interfaces such as + open or pipe2 that accept flags like O_CLOEXEC to create DESC + non-inheritable in the first place. */ int set_cloexec_flag (int desc, bool value) @@ -40,15 +56,98 @@ set_cloexec_flag (int desc, bool value) int newflags = (value ? flags | FD_CLOEXEC : flags & ~FD_CLOEXEC); if (flags == newflags - || fcntl (desc, F_SETFD, newflags) != -1) - return 0; + || fcntl (desc, F_SETFD, newflags) != -1) + return 0; } return -1; #else - return 0; + if (desc < 0) + { + errno = EBADF; + return -1; + } + return dup2 (desc, desc) == desc ? 0 : -1; #endif } + + +/* Duplicates a file handle FD, while marking the copy to be closed + prior to exec or spawn. Returns -1 and sets errno if FD could not + be duplicated. */ + +int dup_cloexec (int fd) +{ + int nfd; + +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ + + /* Native Woe32 API. */ + HANDLE curr_process = GetCurrentProcess (); + HANDLE old_handle = (HANDLE) _get_osfhandle (fd); + HANDLE new_handle; + + if (old_handle == INVALID_HANDLE_VALUE) + { + /* fd is closed, or is open to no handle at all. + We cannot duplicate fd in this case, because _open_osfhandle + fails for an INVALID_HANDLE_VALUE argument. */ + errno = EBADF; + return -1; + } + + if (!DuplicateHandle (curr_process, /* SourceProcessHandle */ + old_handle, /* SourceHandle */ + curr_process, /* TargetProcessHandle */ + (PHANDLE) &new_handle, /* TargetHandle */ + (DWORD) 0, /* DesiredAccess */ + FALSE, /* InheritHandle */ + DUPLICATE_SAME_ACCESS)) /* Options */ + { + errno = EMFILE; + return -1; + } + + nfd = _open_osfhandle ((long) new_handle, O_BINARY | O_NOINHERIT); + if (nfd < 0) + { + CloseHandle (new_handle); + errno = EMFILE; + return -1; + } + +# if REPLACE_FCHDIR + if (0 <= nfd) + result = _gl_register_dup (fd, nfd); +# endif + return nfd; + +#else /* !_WIN32 */ + + /* Unix API. */ + +# ifdef F_DUPFD_CLOEXEC + nfd = fcntl (fd, F_DUPFD_CLOEXEC, 0); +# if REPLACE_FCHDIR + if (0 <= nfd) + result = _gl_register_dup (fd, nfd); +# endif + +# else /* !F_DUPFD_CLOEXEC */ + nfd = dup (fd); + if (0 <= nfd && set_cloexec_flag (nfd, true)) + { + int saved_errno = errno; + close (nfd); + nfd = -1; + errno = saved_errno; + } +# endif /* !F_DUPFD_CLOEXEC */ + + return nfd; + +#endif /* !_WIN32 */ +} diff --git a/lib/cloexec.h b/lib/cloexec.h index c25921d..7333452 100644 --- a/lib/cloexec.h +++ b/lib/cloexec.h @@ -1,2 +1,38 @@ +/* closexec.c - set or clear the close-on-exec descriptor flag + + Copyright (C) 2004, 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/>. + +*/ + #include <stdbool.h> + +/* Set the `FD_CLOEXEC' flag of DESC if VALUE is true, + or clear the flag if VALUE is false. + Return 0 on success, or -1 on error with `errno' set. + + Note that on MingW, this function does NOT protect DESC from being + inherited into spawned children. Instead, either use dup_cloexec + followed by closing the original DESC, or use interfaces such as + open or pipe2 that accept flags like O_CLOEXEC to create DESC + non-inheritable in the first place. */ + int set_cloexec_flag (int desc, bool value); + +/* Duplicates a file handle FD, while marking the copy to be closed + prior to exec or spawn. Returns -1 and sets errno if FD could not + be duplicated. */ + +int dup_cloexec (int fd); diff --git a/lib/w32spawn.h b/lib/w32spawn.h index 375d0cb..693b119 100644 --- a/lib/w32spawn.h +++ b/lib/w32spawn.h @@ -27,6 +27,7 @@ #include <unistd.h> #include <errno.h> +#include "cloexec.h" #include "xalloc.h" /* Duplicates a file handle, making the copy uninheritable. @@ -34,32 +35,11 @@ static int dup_noinherit (int fd) { - HANDLE curr_process = GetCurrentProcess (); - HANDLE old_handle = (HANDLE) _get_osfhandle (fd); - HANDLE new_handle; - int nfd; - - if (old_handle == INVALID_HANDLE_VALUE) - /* fd is closed, or is open to no handle at all. - We cannot duplicate fd in this case, because _open_osfhandle fails for - an INVALID_HANDLE_VALUE argument. */ - return -1; - - if (!DuplicateHandle (curr_process, /* SourceProcessHandle */ - old_handle, /* SourceHandle */ - curr_process, /* TargetProcessHandle */ - (PHANDLE) &new_handle, /* TargetHandle */ - (DWORD) 0, /* DesiredAccess */ - FALSE, /* InheritHandle */ - DUPLICATE_SAME_ACCESS)) /* Options */ - error (EXIT_FAILURE, 0, _("DuplicateHandle failed with error code 0x%08x"), - (unsigned int) GetLastError ()); - - nfd = _open_osfhandle ((long) new_handle, O_BINARY | O_NOINHERIT); - if (nfd < 0) + fd = dup_cloexec (fd); + if (fd < 0 && errno == EMFILE) error (EXIT_FAILURE, errno, _("_open_osfhandle failed")); - return nfd; + return fd; } /* Returns a file descriptor equivalent to FD, except that the resulting file diff --git a/modules/cloexec b/modules/cloexec index 60c04c5..7e41fec 100644 --- a/modules/cloexec +++ b/modules/cloexec @@ -7,6 +7,7 @@ lib/cloexec.h m4/cloexec.m4 Depends-on: +dup2 stdbool configure.ac: diff --git a/modules/cloexec-tests b/modules/cloexec-tests new file mode 100644 index 0000000..38c304c --- /dev/null +++ b/modules/cloexec-tests @@ -0,0 +1,10 @@ +Files: +tests/test-cloexec.c + +Depends-on: + +configure.ac: + +Makefile.am: +TESTS += test-cloexec +check_PROGRAMS += test-cloexec diff --git a/modules/execute b/modules/execute index 88c4fa4..755e3e5 100644 --- a/modules/execute +++ b/modules/execute @@ -8,6 +8,7 @@ lib/w32spawn.h m4/execute.m4 Depends-on: +cloexec dup2 error exit diff --git a/modules/pipe b/modules/pipe index bb1ebee..1d68eeb 100644 --- a/modules/pipe +++ b/modules/pipe @@ -8,6 +8,7 @@ lib/w32spawn.h m4/pipe.m4 Depends-on: +cloexec dup2 environ error diff --git a/tests/test-cloexec.c b/tests/test-cloexec.c new file mode 100644 index 0000000..8df733a --- /dev/null +++ b/tests/test-cloexec.c @@ -0,0 +1,122 @@ +/* Test duplicating non-inheritable file descriptors. + 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 "cloexec.h" + +#include <errno.h> +#include <fcntl.h> +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> + +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ +/* Get declarations of the Win32 API functions. */ +# define WIN32_LEAN_AND_MEAN +# include <windows.h> +#endif + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +/* Return non-zero if FD is open and inheritable across exec/spawn. */ +static int +is_inheritable (int fd) +{ +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ + /* On Win32, the initial state of unassigned standard file + descriptors is that they are open but point to an + INVALID_HANDLE_VALUE, and there is no fcntl. */ + HANDLE h = (HANDLE) _get_osfhandle (fd); + DWORD flags; + if (h == INVALID_HANDLE_VALUE || GetHandleInformation (h, &flags) == 0) + return 0; + return (flags & HANDLE_FLAG_INHERIT) != 0; +#else +# ifndef F_GETFD +# error Please port fcntl to your platform +# endif + int i = fcntl (fd, F_GETFD); + return 0 <= i && (i & FD_CLOEXEC) == 0; +#endif +} + +int +main (void) +{ + const char *file = "test-cloexec.tmp"; + int fd = creat (file, 0600); + int fd2; + + /* Assume std descriptors were provided by invoker. */ + ASSERT (STDERR_FILENO < fd); + ASSERT (is_inheritable (fd)); + + /* Normal use of set_cloexec_flag. */ + ASSERT (set_cloexec_flag (fd, true) == 0); +#if !((defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__) + ASSERT (!is_inheritable (fd)); +#endif + ASSERT (set_cloexec_flag (fd, false) == 0); + ASSERT (is_inheritable (fd)); + + /* Normal use of dup_cloexec. */ + fd2 = dup_cloexec (fd); + ASSERT (fd < fd2); + ASSERT (!is_inheritable (fd2)); + ASSERT (close (fd) == 0); + ASSERT (dup_cloexec (fd2) == fd); + ASSERT (!is_inheritable (fd)); + ASSERT (close (fd2) == 0); + + /* Test error handling. */ + errno = 0; + ASSERT (set_cloexec_flag (-1, false) == -1); + ASSERT (errno == EBADF); + errno = 0; + ASSERT (set_cloexec_flag (10000000, false) == -1); + ASSERT (errno == EBADF); + errno = 0; + ASSERT (set_cloexec_flag (fd2, false) == -1); + ASSERT (errno == EBADF); + errno = 0; + ASSERT (dup_cloexec (-1) == -1); + ASSERT (errno == EBADF); + errno = 0; + ASSERT (dup_cloexec (10000000) == -1); + ASSERT (errno == EBADF); + errno = 0; + ASSERT (dup_cloexec (fd2) == -1); + ASSERT (errno == EBADF); + + /* Clean up. */ + ASSERT (close (fd) == 0); + ASSERT (unlink (file) == 0); + + return 0; +} -- 1.6.5.rc1 >From 173359dbe40ac4381088060b0f26e66529fa0f29 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Sat, 5 Dec 2009 06:19:01 -0700 Subject: [PATCH 2/5] test-dup2: enhance test Ensure that dup2(cloexec_fd, target) returns an inheritable fd. * modules/dup2-tests (Depends-on): Add cloexec. * tests/test-dup2.c (main): Enhance test. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 4 ++++ modules/dup2-tests | 1 + tests/test-dup2.c | 50 ++++++++++++++++++++++++++++++++++++++++++-------- 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index f0c2abd..0c1c870 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-12-05 Eric Blake <e...@byu.net> + test-dup2: enhance test + * modules/dup2-tests (Depends-on): Add cloexec. + * tests/test-dup2.c (main): Enhance test. + cloexec: add dup_cloexec * lib/cloexec.h (dup_cloexec): New prototype. Add copyright header and comments. diff --git a/modules/dup2-tests b/modules/dup2-tests index f11f138..0fb913a 100644 --- a/modules/dup2-tests +++ b/modules/dup2-tests @@ -2,6 +2,7 @@ Files: tests/test-dup2.c Depends-on: +cloexec open configure.ac: diff --git a/tests/test-dup2.c b/tests/test-dup2.c index 32c354d..005160f 100644 --- a/tests/test-dup2.c +++ b/tests/test-dup2.c @@ -25,6 +25,8 @@ #include <stdio.h> #include <stdlib.h> +#include "cloexec.h" + #if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ /* Get declarations of the Win32 API functions. */ # define WIN32_LEAN_AND_MEAN @@ -32,15 +34,15 @@ #endif #define ASSERT(expr) \ - do \ - { \ - if (!(expr)) \ - { \ + do \ + { \ + if (!(expr)) \ + { \ fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ - fflush (stderr); \ - abort (); \ - } \ - } \ + fflush (stderr); \ + abort (); \ + } \ + } \ while (0) /* Return non-zero if FD is open. */ @@ -60,6 +62,28 @@ is_open (int fd) #endif } +/* Return non-zero if FD is open and inheritable across exec/spawn. */ +static int +is_inheritable (int fd) +{ +#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__ + /* On Win32, the initial state of unassigned standard file + descriptors is that they are open but point to an + INVALID_HANDLE_VALUE, and there is no fcntl. */ + HANDLE h = (HANDLE) _get_osfhandle (fd); + DWORD flags; + if (h == INVALID_HANDLE_VALUE || GetHandleInformation (h, &flags) == 0) + return 0; + return (flags & HANDLE_FLAG_INHERIT) != 0; +#else +# ifndef F_GETFD +# error Please port fcntl to your platform +# endif + int i = fcntl (fd, F_GETFD); + return 0 <= i && (i & FD_CLOEXEC) == 0; +#endif +} + int main (void) { @@ -125,8 +149,18 @@ main (void) ASSERT (read (fd, buffer, 1) == 1); ASSERT (*buffer == '2'); + /* Any new fd created by dup2 must not be cloexec. */ + ASSERT (close (fd + 2) == 0); + ASSERT (dup_cloexec (fd) == fd + 1); + ASSERT (!is_inheritable (fd + 1)); + ASSERT (dup2 (fd + 1, fd + 1) == fd + 1); + ASSERT (!is_inheritable (fd + 1)); + ASSERT (dup2 (fd + 1, fd + 2) == fd + 2); + ASSERT (is_inheritable (fd + 2)); + /* Clean up. */ ASSERT (close (fd + 2) == 0); + ASSERT (close (fd + 1) == 0); ASSERT (close (fd) == 0); ASSERT (unlink (file) == 0); -- 1.6.5.rc1 >From 14449a1dacca44c4f933d5fc207c7fa56e6c0fb7 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Sat, 5 Dec 2009 06:36:33 -0700 Subject: [PATCH 3/5] unistd-safer: allow preservation of cloexec status via flag If cloexec is in use, allow the ability to preserve cloexec flag across *_safer functions. * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New prototypes. * lib/dup-safer.c (dup_safer_flag): New function. * lib/fd-safer.c (fd_safer_flag): Likewise. * modules/cloexec (configure.ac): Set witness. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 7 +++++++ lib/dup-safer.c | 32 +++++++++++++++++++++++++++++++- lib/fd-safer.c | 29 +++++++++++++++++++++++++++++ lib/unistd-safer.h | 9 +++++++-- modules/cloexec | 1 + 5 files changed, 75 insertions(+), 3 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0c1c870..20535a0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2009-12-05 Eric Blake <e...@byu.net> + unistd-safer: allow preservation of cloexec status via flag + * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New + prototypes. + * lib/dup-safer.c (dup_safer_flag): New function. + * lib/fd-safer.c (fd_safer_flag): Likewise. + * modules/cloexec (configure.ac): Set witness. + test-dup2: enhance test * modules/dup2-tests (Depends-on): Add cloexec. * tests/test-dup2.c (main): Enhance test. diff --git a/lib/dup-safer.c b/lib/dup-safer.c index 22b96ba..2af8b6a 100644 --- a/lib/dup-safer.c +++ b/lib/dup-safer.c @@ -23,7 +23,6 @@ #include "unistd-safer.h" #include <fcntl.h> - #include <unistd.h> /* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or @@ -40,3 +39,34 @@ dup_safer (int fd) return fd_safer (dup (fd)); #endif } + +#if GNULIB_CLOEXEC + +# include "cloexec.h" + +# ifndef O_CLOEXEC +# define O_CLOEXEC 0 +# endif + +/* Like dup, but do not return STDIN_FILENO, STDOUT_FILENO, or + STDERR_FILENO. If FLAG contains O_CLOEXEC, behave like + fcntl(F_DUPFD_CLOEXEC) rather than fcntl(F_DUPFD). */ + +int +dup_safer_flag (int fd, int flag) +{ + if (flag & O_CLOEXEC) + { +# if defined F_DUPFD_CLOEXEC && !REPLACE_FCHDIR + return fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1); +# else + /* fd_safer_flag calls us back, but eventually the recursion + unwinds and does the right thing. */ + fd = dup_cloexec (fd); + return fd_safer_flag (fd, flag); +# endif + } + return dup_safer (fd); +} + +#endif diff --git a/lib/fd-safer.c b/lib/fd-safer.c index fb99001..d2e1309 100644 --- a/lib/fd-safer.c +++ b/lib/fd-safer.c @@ -48,3 +48,32 @@ fd_safer (int fd) return fd; } + +#if GNULIB_CLOEXEC + +/* Return FD, unless FD would be a copy of standard input, output, or + error; in that case, return a duplicate of FD, closing FD. If FLAG + contains O_CLOEXEC, the returned FD will have close-on-exec + semantics. On failure to duplicate, close FD, set errno, and + return -1. Preserve errno if FD is negative, so that the caller + can always inspect errno when the returned value is negative. + + This function is usefully wrapped around functions that return file + descriptors, e.g., fd_safer_flag (open ("file", O_RDONLY | flag), flag). */ + +int +fd_safer_flag (int fd, int flag) +{ + if (STDIN_FILENO <= fd && fd <= STDERR_FILENO) + { + int f = dup_safer_flag (fd, flag); + int e = errno; + close (fd); + errno = e; + fd = f; + } + + return fd; +} + +#endif /* GNULIB_CLOEXEC */ diff --git a/lib/unistd-safer.h b/lib/unistd-safer.h index 033e857..af7d4ea 100644 --- a/lib/unistd-safer.h +++ b/lib/unistd-safer.h @@ -1,6 +1,6 @@ /* Invoke unistd-like functions, but avoid some glitches. - Copyright (C) 2001, 2003, 2005 Free Software Foundation, Inc. + Copyright (C) 2001, 2003, 2005, 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 @@ -15,8 +15,13 @@ 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 Paul Eggert. */ +/* Written by Paul Eggert and Eric Blake. */ int dup_safer (int); int fd_safer (int); int pipe_safer (int[2]); + +#if GNULIB_CLOEXEC +int dup_safer_flag (int, int); +int fd_safer_flag (int, int); +#endif diff --git a/modules/cloexec b/modules/cloexec index 7e41fec..7d987de 100644 --- a/modules/cloexec +++ b/modules/cloexec @@ -12,6 +12,7 @@ stdbool configure.ac: gl_CLOEXEC +gl_MODULE_INDICATOR([cloexec]) Makefile.am: -- 1.6.5.rc1 >From f2fdd542de54c78c89fd8b50b76921cf31fdb518 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Mon, 16 Nov 2009 16:09:42 -0700 Subject: [PATCH 4/5] stdlib-safer: preserve cloexec flag for mkostemp[s] mkostemp_safer(templ,O_CLOEXEC) did not always guarantee cloexec. * lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new fd_safer_flag. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 4 ++++ lib/mkstemp-safer.c | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 20535a0..21c86be 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,9 @@ 2009-12-05 Eric Blake <e...@byu.net> + stdlib-safer: preserve cloexec flag for mkostemp[s] + * lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new + fd_safer_flag. + unistd-safer: allow preservation of cloexec status via flag * lib/unistd-safer.h (dup_safer_flag, fd_safer_flag): New prototypes. diff --git a/lib/mkstemp-safer.c b/lib/mkstemp-safer.c index 95d315b..ee242f3 100644 --- a/lib/mkstemp-safer.c +++ b/lib/mkstemp-safer.c @@ -39,7 +39,7 @@ mkstemp_safer (char *templ) int mkostemp_safer (char *templ, int flags) { - return fd_safer (mkostemp (templ, flags)); + return fd_safer_flag (mkostemp (templ, flags), flags); } #endif @@ -49,7 +49,7 @@ mkostemp_safer (char *templ, int flags) int mkostemps_safer (char *templ, int suffixlen, int flags) { - return fd_safer (mkostemps (templ, suffixlen, flags)); + return fd_safer_flag (mkostemps (templ, suffixlen, flags), flags); } #endif -- 1.6.5.rc1 >From 6ed67a15a265e213d2cbaac25ccc995fddf73ac6 Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Sat, 5 Dec 2009 06:39:09 -0700 Subject: [PATCH 5/5] pipe2-safer: new module pipe2 deserves a *_safer variant. It also makes the code in pipe.c look simpler. * modules/pipe2-safer: New file. * lib/unistd-safer.h (pipe2_safer): New prototype. * lib/unistd--.h (pipe2): New wrapper. * lib/pipe-safer.c (pipe2_safer): New function. * modules/pipe (Depends-on): Add pipe2-safer. * lib/pipe.c (create_pipe) [WIN32]: Let pipe2_safer do the work. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 8 ++++++++ lib/pipe-safer.c | 50 +++++++++++++++++++++++++++++++++++++++----------- lib/pipe.c | 8 ++------ lib/unistd--.h | 7 ++++++- lib/unistd-safer.h | 4 ++++ modules/pipe | 1 + modules/pipe2-safer | 24 ++++++++++++++++++++++++ 7 files changed, 84 insertions(+), 18 deletions(-) create mode 100644 modules/pipe2-safer diff --git a/ChangeLog b/ChangeLog index 21c86be..90d6eb0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2009-12-05 Eric Blake <e...@byu.net> + pipe2-safer: new module + * modules/pipe2-safer: New file. + * lib/unistd-safer.h (pipe2_safer): New prototype. + * lib/unistd--.h (pipe2): New wrapper. + * lib/pipe-safer.c (pipe2_safer): New function. + * modules/pipe (Depends-on): Add pipe2-safer. + * lib/pipe.c (create_pipe) [WIN32]: Let pipe2_safer do the work. + stdlib-safer: preserve cloexec flag for mkostemp[s] * lib/mkstemp-safer.c (mkostemp_safer, mkostemps_safer): Use new fd_safer_flag. diff --git a/lib/pipe-safer.c b/lib/pipe-safer.c index 0fc6850..fda5c52 100644 --- a/lib/pipe-safer.c +++ b/lib/pipe-safer.c @@ -1,5 +1,5 @@ /* Invoke pipe, but avoid some glitches. - Copyright (C) 2005, 2006 Free Software Foundation, Inc. + Copyright (C) 2005, 2006, 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 @@ -35,16 +35,16 @@ pipe_safer (int fd[2]) { int i; for (i = 0; i < 2; i++) - { - fd[i] = fd_safer (fd[i]); - if (fd[i] < 0) - { - int e = errno; - close (fd[1 - i]); - errno = e; - return -1; - } - } + { + fd[i] = fd_safer (fd[i]); + if (fd[i] < 0) + { + int e = errno; + close (fd[1 - i]); + errno = e; + return -1; + } + } return 0; } @@ -54,3 +54,31 @@ pipe_safer (int fd[2]) return -1; } + +#if GNULIB_PIPE2_SAFER +/* Like pipe2, but ensure that neither of the file descriptors is + STDIN_FILENO, STDOUT_FILENO, or STDERR_FILENO. */ + +int +pipe2_safer (int fd[2], int flags) +{ + if (pipe2 (fd, flags) == 0) + { + int i; + for (i = 0; i < 2; i++) + { + fd[i] = fd_safer_flag (fd[i], flags); + if (fd[i] < 0) + { + int e = errno; + close (fd[1 - i]); + errno = e; + return -1; + } + } + + return 0; + } + return -1; +} +#endif /* GNULIB_PIPE2 */ diff --git a/lib/pipe.c b/lib/pipe.c index d17c9bf..c3db1b5 100644 --- a/lib/pipe.c +++ b/lib/pipe.c @@ -133,14 +133,10 @@ create_pipe (const char *progname, prog_argv = prepare_spawn (prog_argv); if (pipe_stdout) - if (pipe2 (ifd, O_BINARY | O_NOINHERIT) < 0 - || (ifd[0] = fd_safer_noinherit (ifd[0])) < 0 - || (ifd[1] = fd_safer_noinherit (ifd[1])) < 0) + if (pipe2_safer (ifd, O_BINARY | O_CLOEXEC) < 0) error (EXIT_FAILURE, errno, _("cannot create pipe")); if (pipe_stdin) - if (pipe2 (ofd, O_BINARY | O_NOINHERIT) < 0 - || (ofd[0] = fd_safer_noinherit (ofd[0])) < 0 - || (ofd[1] = fd_safer_noinherit (ofd[1])) < 0) + if (pipe2_safer (ofd, O_BINARY | O_CLOEXEC) < 0) error (EXIT_FAILURE, errno, _("cannot create pipe")); /* Data flow diagram: * diff --git a/lib/unistd--.h b/lib/unistd--.h index 1a7fd78..6f1ebaf 100644 --- a/lib/unistd--.h +++ b/lib/unistd--.h @@ -1,6 +1,6 @@ /* Like unistd.h, but redefine some names to avoid glitches. - Copyright (C) 2005 Free Software Foundation, Inc. + Copyright (C) 2005, 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 @@ -25,3 +25,8 @@ #undef pipe #define pipe pipe_safer + +#if GNULIB_PIPE2_SAFER +# undef pipe2 +# define pipe2 pipe2_safer +#endif diff --git a/lib/unistd-safer.h b/lib/unistd-safer.h index af7d4ea..70cc699 100644 --- a/lib/unistd-safer.h +++ b/lib/unistd-safer.h @@ -25,3 +25,7 @@ int pipe_safer (int[2]); int dup_safer_flag (int, int); int fd_safer_flag (int, int); #endif + +#if GNULIB_PIPE2_SAFER +int pipe2_safer (int[2], int); +#endif diff --git a/modules/pipe b/modules/pipe index 1d68eeb..922bda1 100644 --- a/modules/pipe +++ b/modules/pipe @@ -17,6 +17,7 @@ fatal-signal gettext-h open pipe2 +pipe2-safer spawn posix_spawnp posix_spawn_file_actions_init diff --git a/modules/pipe2-safer b/modules/pipe2-safer new file mode 100644 index 0000000..eb59487 --- /dev/null +++ b/modules/pipe2-safer @@ -0,0 +1,24 @@ +Description: +pipe2_safer() function: create a pipe, with specific opening flags, +without clobbering std{in,out,err}. + +Files: + +Depends-on: +cloexec +pipe2 +unistd-safer + +configure.ac: +gl_MODULE_INDICATOR([pipe2-safer]) + +Makefile.am: + +Include: +"unistd-safer.h" + +License: +GPL + +Maintainer: +Eric Blake -- 1.6.5.rc1