From 990fa9d50c060b3acf2bcf1f1e0578b6e975310b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 14 Aug 2017 13:04:46 -0700 Subject: [PATCH] open: support O_CLOEXEC * NEWS, doc/posix-functions/open.texi: * doc/posix-functions/openat.texi: Document this. * lib/fcntl.in.h (O_CLOEXEC): Default to a nonzero value. (GNULIB_defined_O_CLOEXEC): New symbol. * lib/open.c: Include cloexec.h. (open): Support O_CLOEXEC. * lib/openat.c: Include cloexec.h. (rpl_openat): Support O_CLOEXEC. * lib/popen-safer.c: Do not include cloexec.h. (open_noinherit): Remove. (popen_safer): Use O_CLOEXEC instead of set_cloexec_flag. * lib/save-cwd.c: Do not include cloexec.h. (save_cwd): Use O_CLOEXEC instead of set_cloexec_flag. * m4/open-cloexec.m4: New file. * m4/open.m4 (gl_FUNC_OPEN): Require gl_PREPROC_O_CLOEXEC. Replace 'open' if O_CLOEXEC is not present. * m4/openat.m4 (gl_FUNC_OPENAT): Require gl_PREPROC_O_CLOEXEC. Replace 'openat' if O_CLOEXEC is not present. * modules/freopen (Depends-on): Depend on 'open' if replacing freopen. * modules/open (Files): Add m4/open-cloexec.m4. (Depends-on): Depend on cloexec if replacing 'open'. * modules/openat (Files): Add m4/open-cloexec.m4. (Depends-on): Depend on cloexec if replacing openat. * modules/popen-safer (Depends-on): Remove cloexec. * modules/save-cwd (Depends-on): Remove cloexec, and add fd-safer-flag and 'open'. --- ChangeLog | 31 +++++++++++++++++++++++++++++ NEWS | 4 ++++ doc/posix-functions/open.texi | 6 ++++++ doc/posix-functions/openat.texi | 6 ++++++ lib/fcntl.in.h | 5 ++++- lib/open.c | 29 ++++++++++++++++++++++++++- lib/openat.c | 32 ++++++++++++++++++++++++++++-- lib/popen-safer.c | 35 +-------------------------------- lib/save-cwd.c | 6 ++---- m4/open-cloexec.m4 | 21 ++++++++++++++++++++ m4/open.m4 | 6 +++++- m4/openat.m4 | 8 +++++--- modules/freopen | 1 + modules/open | 2 ++ modules/openat | 2 ++ modules/popen-safer | 1 - modules/save-cwd | 5 +++-- 17 files changed, 151 insertions(+), 49 deletions(-) create mode 100644 m4/open-cloexec.m4 diff --git a/ChangeLog b/ChangeLog index a07e17251d..7f1a5c2e9c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,34 @@ +2017-08-14 Paul Eggert + + + open: support O_CLOEXEC + * NEWS, doc/posix-functions/open.texi: + * doc/posix-functions/openat.texi: Document this. + * lib/fcntl.in.h (O_CLOEXEC): Default to a nonzero value. + (GNULIB_defined_O_CLOEXEC): New symbol. + * lib/open.c: Include cloexec.h. + (open): Support O_CLOEXEC. + * lib/openat.c: Include cloexec.h. + (rpl_openat): Support O_CLOEXEC. + * lib/popen-safer.c: Do not include cloexec.h. + (open_noinherit): Remove. + (popen_safer): Use O_CLOEXEC instead of set_cloexec_flag. + * lib/save-cwd.c: Do not include cloexec.h. + (save_cwd): Use O_CLOEXEC instead of set_cloexec_flag. + * m4/open-cloexec.m4: New file. + * m4/open.m4 (gl_FUNC_OPEN): Require gl_PREPROC_O_CLOEXEC. + Replace 'open' if O_CLOEXEC is not present. + * m4/openat.m4 (gl_FUNC_OPENAT): Require gl_PREPROC_O_CLOEXEC. + Replace 'openat' if O_CLOEXEC is not present. + * modules/freopen (Depends-on): Depend on 'open' if replacing freopen. + * modules/open (Files): Add m4/open-cloexec.m4. + (Depends-on): Depend on cloexec if replacing 'open'. + * modules/openat (Files): Add m4/open-cloexec.m4. + (Depends-on): Depend on cloexec if replacing openat. + * modules/popen-safer (Depends-on): Remove cloexec. + * modules/save-cwd (Depends-on): Remove cloexec, and add + fd-safer-flag and 'open'. + 2017-08-13 Paul Eggert reallocarray: minor fixes diff --git a/NEWS b/NEWS index 8fb2c54dd0..d3a300a30e 100644 --- a/NEWS +++ b/NEWS @@ -42,6 +42,10 @@ User visible incompatible changes Date Modules Changes +2017-08-14 fcntl-h This module now defaults O_CLOEXEC to a nonzero + value instead of to 0, as the 'open' and + 'openat' modules now emulate O_CLOEXEC. + 2017-07-23 strftime This module is renamed to 'nstrftime'. 2017-05-19 closeout close_stdout longer closes stderr when addresses diff --git a/doc/posix-functions/open.texi b/doc/posix-functions/open.texi index b8cda062fe..f1565c31a5 100644 --- a/doc/posix-functions/open.texi +++ b/doc/posix-functions/open.texi @@ -9,6 +9,9 @@ Gnulib module: open, fchdir Portability problems fixed by the Gnulib module open: @itemize @item +Some platforms do not support @code{O_CLOEXEC}: +Solaris 10, probably many others. +@item On platforms where @code{off_t} is a 32-bit type, @code{open} may not work correctly with files larger than 2 GB. (Cf. @code{AC_SYS_LARGEFILE}.) @item @@ -35,6 +38,9 @@ read-only descriptor for directories. Portability problems not fixed by Gnulib: @itemize @item +The Gnulib replacement for @code{O_CLOEXEC} is not atomic, and so is +not safe in the presence of multiple threads or signal handlers. +@item @code{open ("symlink", O_NOFOLLOW ...)} fails with @code{errno} set to @code{EMLINK} instead of the POSIX-required @code{ELOOP} on some platforms: diff --git a/doc/posix-functions/openat.texi b/doc/posix-functions/openat.texi index f93c98c194..9f7632b1b9 100644 --- a/doc/posix-functions/openat.texi +++ b/doc/posix-functions/openat.texi @@ -14,6 +14,9 @@ glibc 2.3.6, Mac OS X 10.5, FreeBSD 6.0, NetBSD 5.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Cygwin 1.5.x, mingw, MSVC 14, Interix 3.5, BeOS. But the replacement function is not safe to be used in libraries and is not multithread-safe. @item +Some platforms do not support @code{O_CLOEXEC}: +Solaris 10. +@item On platforms where @code{off_t} is a 32-bit type, @code{open} may not work correctly with files larger than 2 GB. (Cf. @code{AC_SYS_LARGEFILE}.) @item @@ -26,6 +29,9 @@ Solaris 9. Portability problems not fixed by Gnulib: @itemize @item +The Gnulib replacement for @code{O_CLOEXEC} is not atomic, and so is +not safe in the presence of multiple threads or signal handlers. +@item @code{openat (fd, "symlink", O_NOFOLLOW ...)} fails with @code{errno} set to @code{EMLINK} instead of the POSIX-required @code{ELOOP} on some platforms: diff --git a/lib/fcntl.in.h b/lib/fcntl.in.h index 4a1d40af6d..076d1ac34a 100644 --- a/lib/fcntl.in.h +++ b/lib/fcntl.in.h @@ -213,7 +213,10 @@ _GL_WARN_ON_USE (openat, "openat is not portable - " #endif #ifndef O_CLOEXEC -# define O_CLOEXEC 0 +# define O_CLOEXEC 0x40000000 /* Try to not collide with system O_* flags. */ +# define GNULIB_defined_O_CLOEXEC 1 +#else +# define GNULIB_defined_O_CLOEXEC 0 #endif #ifndef O_DIRECT diff --git a/lib/open.c b/lib/open.c index 193dc45f6d..c62f02b145 100644 --- a/lib/open.c +++ b/lib/open.c @@ -38,6 +38,8 @@ orig_open (const char *filename, int flags, mode_t mode) this include because of the preliminary #include above. */ #include "fcntl.h" +#include "cloexec.h" + #include #include #include @@ -52,6 +54,13 @@ orig_open (const char *filename, int flags, mode_t mode) int open (const char *filename, int flags, ...) { + /* 0 = unknown, 1 = yes, -1 = no. */ +#if GNULIB_defined_O_CLOEXEC + int have_cloexec = -1; +#else + static int have_cloexec; +#endif + mode_t mode; int fd; @@ -115,7 +124,25 @@ open (const char *filename, int flags, ...) } #endif - fd = orig_open (filename, flags, mode); + fd = orig_open (filename, + flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode); + + if (flags & O_CLOEXEC) + { + if (! have_cloexec) + { + if (0 <= fd) + have_cloexec = 1; + else if (errno == EINVAL) + { + fd = orig_open (filename, flags & ~O_CLOEXEC, mode); + have_cloexec = -1; + } + } + if (have_cloexec < 0 && 0 <= fd) + set_cloexec_flag (fd, true); + } + #if REPLACE_FCHDIR /* Implementing fchdir and fdopendir requires the ability to open a diff --git a/lib/openat.c b/lib/openat.c index e739b7143c..f393010148 100644 --- a/lib/openat.c +++ b/lib/openat.c @@ -41,6 +41,8 @@ orig_openat (int fd, char const *filename, int flags, mode_t mode) #include "openat.h" +#include "cloexec.h" + #include #include #include @@ -50,10 +52,18 @@ orig_openat (int fd, char const *filename, int flags, mode_t mode) #if HAVE_OPENAT -/* Like openat, but work around Solaris 9 bugs with trailing slash. */ +/* Like openat, but support O_CLOEXEC and work around Solaris 9 bugs + with trailing slash. */ int rpl_openat (int dfd, char const *filename, int flags, ...) { + /* 0 = unknown, 1 = yes, -1 = no. */ +#if GNULIB_defined_O_CLOEXEC + int have_cloexec = -1; +#else + static int have_cloexec; +#endif + mode_t mode; int fd; @@ -103,7 +113,25 @@ rpl_openat (int dfd, char const *filename, int flags, ...) } # endif - fd = orig_openat (dfd, filename, flags, mode); + fd = orig_openat (dfd, filename, + flags & ~(have_cloexec <= 0 ? O_CLOEXEC : 0), mode); + + if (flags & O_CLOEXEC) + { + if (! have_cloexec) + { + if (0 <= fd) + have_cloexec = 1; + else if (errno == EINVAL) + { + fd = orig_openat (dfd, filename, flags & ~O_CLOEXEC, mode); + have_cloexec = -1; + } + } + if (have_cloexec < 0 && 0 <= fd) + set_cloexec_flag (fd, true); + } + # if OPEN_TRAILING_SLASH_BUG /* If the filename ends in a slash and fd does not refer to a directory, diff --git a/lib/popen-safer.c b/lib/popen-safer.c index 503780f351..5614f263b1 100644 --- a/lib/popen-safer.c +++ b/lib/popen-safer.c @@ -25,39 +25,6 @@ #include #include -#include "cloexec.h" - -/* Like open (name, flags | O_CLOEXEC), although not necessarily - atomic. FLAGS must not include O_CREAT. */ - -static int -open_noinherit (char const *name, int flags) -{ - int fd; -#if O_CLOEXEC - /* 0 = unknown, 1 = yes, -1 = no. */ - static int have_cloexec; - if (have_cloexec >= 0) - { - fd = open (name, flags | O_CLOEXEC); - if (have_cloexec == 0 && (0 <= fd || errno == EINVAL)) - have_cloexec = (0 <= fd ? 1 : -1); - if (have_cloexec == 1) - return fd; - } -#endif - - fd = open (name, flags); - if (0 <= fd && set_cloexec_flag (fd, true) != 0) - { - int saved_errno = errno; - close (fd); - fd = -1; - errno = saved_errno; - } - return fd; -} - /* Like popen, but do not return stdin, stdout, or stderr. */ FILE * @@ -72,7 +39,7 @@ popen_safer (char const *cmd, char const *mode) call (even though this puts more pressure on open fds), so that the original fd created by popen is safe. */ FILE *fp; - int fd = open_noinherit ("/dev/null", O_RDONLY); + int fd = open ("/dev/null", O_RDONLY | O_CLOEXEC); if (0 <= fd && fd <= STDERR_FILENO) { /* Maximum recursion depth is 3. */ diff --git a/lib/save-cwd.c b/lib/save-cwd.c index 11416cee2a..b82a00a01d 100644 --- a/lib/save-cwd.c +++ b/lib/save-cwd.c @@ -30,7 +30,6 @@ #include "chdir-long.h" #include "unistd--.h" -#include "cloexec.h" #if GNULIB_FCNTL_SAFER # include "fcntl--.h" @@ -64,16 +63,15 @@ save_cwd (struct saved_cwd *cwd) { cwd->name = NULL; - cwd->desc = open (".", O_SEARCH); + cwd->desc = open (".", O_SEARCH | O_CLOEXEC); if (!GNULIB_FCNTL_SAFER) - cwd->desc = fd_safer (cwd->desc); + cwd->desc = fd_safer_flag (cwd->desc, O_CLOEXEC); if (cwd->desc < 0) { cwd->name = getcwd (NULL, 0); return cwd->name ? 0 : -1; } - set_cloexec_flag (cwd->desc, true); return 0; } diff --git a/m4/open-cloexec.m4 b/m4/open-cloexec.m4 new file mode 100644 index 0000000000..897af66910 --- /dev/null +++ b/m4/open-cloexec.m4 @@ -0,0 +1,21 @@ +# Test whether O_CLOEXEC is defined. + +dnl Copyright 2017 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_PREPROC_O_CLOEXEC], +[ + AC_CACHE_CHECK([for O_CLOEXEC], + [gl_cv_macro_O_CLOEXEC], + [AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[#include + #ifndef O_CLOEXEC + choke me; + #endif + ]], + [[return O_CLOEXEC;]])], + [gl_cv_macro_O_CLOEXEC=yes], + [gl_cv_macro_O_CLOEXEC=no])]) +]) diff --git a/m4/open.m4 b/m4/open.m4 index 2a869dc6b7..68253e15ff 100644 --- a/m4/open.m4 +++ b/m4/open.m4 @@ -1,4 +1,4 @@ -# open.m4 serial 14 +# open.m4 serial 15 dnl Copyright (C) 2007-2017 Free Software Foundation, Inc. dnl This file is free software; the Free Software Foundation dnl gives unlimited permission to copy and/or distribute it, @@ -7,6 +7,7 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_FUNC_OPEN], [ AC_REQUIRE([AC_CANONICAL_HOST]) + AC_REQUIRE([gl_PREPROC_O_CLOEXEC]) case "$host_os" in mingw* | pw*) REPLACE_OPEN=1 @@ -15,6 +16,9 @@ AC_DEFUN([gl_FUNC_OPEN], dnl open("foo/") should not create a file when the file name has a dnl trailing slash. FreeBSD only has the problem on symlinks. AC_CHECK_FUNCS_ONCE([lstat]) + if test "$gl_cv_macro_O_CLOEXEC" != yes; then + REPLACE_OPEN=1 + fi AC_CACHE_CHECK([whether open recognizes a trailing slash], [gl_cv_func_open_slash], [# Assume that if we have lstat, we can also check symlinks. diff --git a/m4/openat.m4 b/m4/openat.m4 index 55b946f96e..d0d244a0f4 100644 --- a/m4/openat.m4 +++ b/m4/openat.m4 @@ -1,4 +1,4 @@ -# serial 45 +# serial 46 # See if we need to use our replacement for Solaris' openat et al functions. dnl Copyright (C) 2004-2017 Free Software Foundation, Inc. @@ -14,10 +14,12 @@ AC_DEFUN([gl_FUNC_OPENAT], AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) AC_CHECK_FUNCS_ONCE([openat]) AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) - case $ac_cv_func_openat+$gl_cv_func_lstat_dereferences_slashed_symlink in - yes+*yes) + AC_REQUIRE([gl_PREPROC_O_CLOEXEC]) + case $ac_cv_func_openat+$gl_cv_func_lstat_dereferences_slashed_symlink+$gl_cv_macro_O_CLOEXEC in + yes+*yes+yes) ;; yes+*) + # Solaris 10 lacks O_CLOEXEC. # Solaris 9 has *at functions, but uniformly mishandles trailing # slash in all of them. REPLACE_OPENAT=1 diff --git a/modules/freopen b/modules/freopen index adb4bbcbc5..41236bd533 100644 --- a/modules/freopen +++ b/modules/freopen @@ -7,6 +7,7 @@ m4/freopen.m4 Depends-on: fcntl-h [test $REPLACE_FREOPEN = 1] +open [test $REPLACE_FREOPEN = 1] stdio largefile diff --git a/modules/open b/modules/open index 515170fc3f..62c1a05954 100644 --- a/modules/open +++ b/modules/open @@ -4,11 +4,13 @@ open() function: open a descriptor to a file. Files: lib/open.c m4/open.m4 +m4/open-cloexec.m4 m4/mode_t.m4 Depends-on: fcntl-h largefile +cloexec [test $REPLACE_OPEN = 1] fstat [test $REPLACE_OPEN = 1] stat [test $REPLACE_OPEN = 1] diff --git a/modules/openat b/modules/openat index 0db086a766..66fa3b4f46 100644 --- a/modules/openat +++ b/modules/openat @@ -4,6 +4,7 @@ openat() function: Open a file at a directory. Files: lib/openat.c m4/openat.m4 +m4/open-cloexec.m4 m4/lstat.m4 m4/mode_t.m4 @@ -14,6 +15,7 @@ largefile openat-h [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1] stdbool [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1] sys_stat [test $HAVE_OPENAT = 0 || test $REPLACE_OPENAT = 1] +cloexec [test $REPLACE_OPENAT = 1] fstat [test $REPLACE_OPENAT = 1] at-internal [test $HAVE_OPENAT = 0] dosname [test $HAVE_OPENAT = 0] diff --git a/modules/popen-safer b/modules/popen-safer index 3793976322..4c43037042 100644 --- a/modules/popen-safer +++ b/modules/popen-safer @@ -7,7 +7,6 @@ lib/stdio-safer.h lib/popen-safer.c Depends-on: -cloexec open popen unistd-safer diff --git a/modules/save-cwd b/modules/save-cwd index 876826b0bb..52bf3311df 100644 --- a/modules/save-cwd +++ b/modules/save-cwd @@ -8,9 +8,10 @@ m4/save-cwd.m4 Depends-on: chdir-long -cloexec -getcwd-lgpl fchdir +fd-safer-flag +getcwd-lgpl +open stdbool unistd-safer -- 2.39.5