From: Eric Blake Date: Sat, 19 Sep 2009 13:12:15 +0000 (-0600) Subject: openat: allow return of fd 0 X-Git-Tag: v0.1~5460 X-Git-Url: http://erislabs.org.uk/gitweb/?a=commitdiff_plain;h=996f76cd71a98365044457d94b0d87454bcd4deb;p=gnulib.git openat: allow return of fd 0 Partially reverts patch fc33350 from 2009-09-02. * modules/chdir-long (Depends-on): Relax openat-safer to openat. * modules/save-cwd (Depends-on): Replace fcntl-safer with unistd-safer. * lib/chdir-long.c (includes): Replace "fcntl--.h" with ; this module does not leak fds. * lib/openat.c (includes): Do not use "fcntl_safer"; plain openat must be allowed to return 0, leaving openat_safer to add the safety. (openat_permissive): Avoid writing to just-opened fd 2 if restoring the current directory fails. * lib/openat-die.c (openat_restore_fail): Add comment. * lib/save-cwd.c (includes): Make "fcntl--.h" conditional. (save_cwd): Guarantee safe fd, but without use of open_safer. * tests/test-openat.c: New test. * modules/openat-tests (Files, Makefile.am): Distribute and build new file. Signed-off-by: Eric Blake --- diff --git a/ChangeLog b/ChangeLog index a514b3414..3e8e24b6e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2009-09-19 Eric Blake + openat: allow return of fd 0 + * modules/chdir-long (Depends-on): Relax openat-safer to openat. + * modules/save-cwd (Depends-on): Replace fcntl-safer with + unistd-safer. + * lib/chdir-long.c (includes): Replace "fcntl--.h" with + ; this module does not leak fds. + * lib/openat.c (includes): Do not use "fcntl_safer"; plain openat + must be allowed to return 0, leaving openat_safer to add the + safety. + (openat_permissive): Avoid writing to just-opened fd 2 if + restoring the current directory fails. + * lib/openat-die.c (openat_restore_fail): Add comment. + * lib/save-cwd.c (includes): Make "fcntl--.h" conditional. + (save_cwd): Guarantee safe fd, but without use of open_safer. + * tests/test-openat.c: New test. + * modules/openat-tests (Files, Makefile.am): Distribute and build + new file. + relocatable-prog-wrapper: fix build * modules/relocatable-prog-wrapper (Files): Update name of canonicalize m4 file, broken on 2009-09-17. diff --git a/lib/chdir-long.c b/lib/chdir-long.c index ba47d5997..afe018d16 100644 --- a/lib/chdir-long.c +++ b/lib/chdir-long.c @@ -20,19 +20,23 @@ #include "chdir-long.h" +#include +#include +#include #include #include #include -#include #include -#include - -#include "fcntl--.h" #ifndef PATH_MAX # error "compile this file only if your system defines PATH_MAX" #endif +/* The results of openat() in this file are not leaked to any + single-threaded code that could use stdio. + FIXME - if the kernel ever adds support for multi-thread safety for + avoiding standard fds, then we should use openat_safer. */ + struct cd_buf { int fd; diff --git a/lib/openat-die.c b/lib/openat-die.c index 677f3e02c..a507eac10 100644 --- a/lib/openat-die.c +++ b/lib/openat-die.c @@ -40,6 +40,11 @@ openat_save_fail (int errnum) abort (); } + +/* Exit with an error about failure to restore the working directory + during an openat emulation. The caller must ensure that fd 2 is + not a just-opened fd, even when openat_safer is not in use. */ + void openat_restore_fail (int errnum) { diff --git a/lib/openat.c b/lib/openat.c index e1471b8c0..d22d830b1 100644 --- a/lib/openat.c +++ b/lib/openat.c @@ -28,13 +28,6 @@ #include "openat-priv.h" #include "save-cwd.h" -/* We can't use "fcntl--.h", so that openat_safer does not interfere. */ -#if GNULIB_FCNTL_SAFER -# include "fcntl-safer.h" -# undef open -# define open open_safer -#endif - /* Replacement for Solaris' openat function. First, try to simulate it via open ("/proc/self/fd/FD/FILE"). @@ -124,7 +117,13 @@ openat_permissive (int fd, char const *file, int flags, mode_t mode, if (save_ok && restore_cwd (&saved_cwd) != 0) { if (! cwd_errno) - openat_restore_fail (errno); + { + /* Don't write a message to just-created fd 2. */ + saved_errno = errno; + if (err == STDERR_FILENO) + close (err); + openat_restore_fail (saved_errno); + } *cwd_errno = errno; } } diff --git a/lib/save-cwd.c b/lib/save-cwd.c index e158e8b6b..7c2eb6d94 100644 --- a/lib/save-cwd.c +++ b/lib/save-cwd.c @@ -1,6 +1,6 @@ /* save-cwd.c -- Save and restore current working directory. - Copyright (C) 1995, 1997, 1998, 2003, 2004, 2005, 2006 Free + Copyright (C) 1995, 1997, 1998, 2003, 2004, 2005, 2006, 2009 Free Software Foundation, Inc. This program is free software: you can redistribute it and/or modify @@ -23,15 +23,21 @@ #include "save-cwd.h" #include +#include #include #include #include -#include #include "chdir-long.h" -#include "fcntl--.h" +#include "unistd--.h" #include "xgetcwd.h" +#if GNULIB_FCNTL_SAFER +# include "fcntl--.h" +#else +# define GNULIB_FCNTL_SAFER 0 +#endif + /* On systems without the fchdir function (WOE), pretend that open always returns -1 so that save_cwd resorts to using xgetcwd. Since chdir_long requires fchdir, use chdir instead. */ @@ -70,6 +76,8 @@ save_cwd (struct saved_cwd *cwd) cwd->name = NULL; cwd->desc = open (".", O_RDONLY); + if (!GNULIB_FCNTL_SAFER) + cwd->desc = fd_safer (cwd->desc); if (cwd->desc < 0) { cwd->name = xgetcwd (); diff --git a/modules/chdir-long b/modules/chdir-long index cdcb9eb70..4025b45af 100644 --- a/modules/chdir-long +++ b/modules/chdir-long @@ -10,7 +10,7 @@ Depends-on: atexit fchdir fcntl-h -openat-safer +openat memchr mempcpy memrchr diff --git a/modules/openat-tests b/modules/openat-tests index a82a4f33e..54d0c61d7 100644 --- a/modules/openat-tests +++ b/modules/openat-tests @@ -1,5 +1,6 @@ Files: tests/test-rmdir.h +tests/test-openat.c tests/test-unlinkat.c Depends-on: @@ -8,6 +9,7 @@ configure.ac: AC_CHECK_FUNCS_ONCE([symlink]) Makefile.am: -TESTS += test-unlinkat -check_PROGRAMS += test-unlinkat +TESTS += test-openat test-unlinkat +check_PROGRAMS += test-openat test-unlinkat +test_openat_LDADD = $(LDADD) @LIBINTL@ test_unlinkat_LDADD = $(LDADD) @LIBINTL@ diff --git a/modules/save-cwd b/modules/save-cwd index 18c43b9ef..46a1276ab 100644 --- a/modules/save-cwd +++ b/modules/save-cwd @@ -8,9 +8,9 @@ m4/save-cwd.m4 Depends-on: chdir-long -fcntl-safer -xgetcwd stdbool +unistd-safer +xgetcwd configure.ac: gl_SAVE_CWD diff --git a/tests/test-openat.c b/tests/test-openat.c new file mode 100644 index 000000000..8fa8f83c4 --- /dev/null +++ b/tests/test-openat.c @@ -0,0 +1,60 @@ +/* Test that openat works. + 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 . */ + +/* Written by Eric Blake , 2009. */ + +#include + +#include + +#include +#include +#include + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +int +main () +{ + /* FIXME - add more tests. For example, share /dev/null and + trailing slash tests with test-open, and do more checks for + proper fd handling. */ + + /* Check that even when *-safer modules are in use, plain openat can + land in fd 0. Do this test last, since it is destructive to + stdin. */ + ASSERT (close (STDIN_FILENO) == 0); + ASSERT (openat (AT_FDCWD, ".", O_RDONLY) == STDIN_FILENO); + { + int dfd = open (".", O_RDONLY); + ASSERT (STDIN_FILENO < dfd); + ASSERT (chdir ("..") == 0); + ASSERT (close (STDIN_FILENO) == 0); + ASSERT (openat (dfd, ".", O_RDONLY) == STDIN_FILENO); + ASSERT (close (dfd) == 0); + } + return 0; +}