From 48e988340f85e568ceb9ac1f4bf5824fddf1fd0d Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Sat, 7 Nov 2009 21:34:32 -0700 Subject: [PATCH] open: detect FreeBSD bug open("link-to-file/", O_RDONLY) mistakenly succeeds. The previous patch was enough to fix utimens when no fd is involved, but this is necessary for futimens to pass. * m4/open.m4 (gl_FUNC_OPEN): Also detect FreeBSD bug with slash on symlink. * doc/posix-functions/open.texi (open): Document the bug. * doc/posix-functions/utimes.texi (utimes): Likewise. * tests/test-open.h (test_open): Add parameters, and test symlink handling. * tests/test-open.c (main): Adjust caller. * tests/test-fcntl-safer.c (main): Likewise. * modules/open-tests (Depends-on): Add stdbool, symlink. * modules/fcntl-safer-tests (Depends-on): Likewise. * tests/test-openat.c (main): Add test-open tests. Signed-off-by: Eric Blake --- ChangeLog | 13 ++++++++++ doc/posix-functions/open.texi | 2 +- doc/posix-functions/utimes.texi | 2 +- m4/open.m4 | 17 ++++++++++--- modules/fcntl-safer-tests | 2 ++ modules/open-tests | 3 ++- tests/test-fcntl-safer.c | 20 ++++++++++++++- tests/test-open.c | 20 ++++++++++++++- tests/test-open.h | 56 +++++++++++++++++++++-------------------- tests/test-openat.c | 54 +++++++++++++++++++++++++++++++-------- 10 files changed, 143 insertions(+), 46 deletions(-) diff --git a/ChangeLog b/ChangeLog index c223b02c3..57848e469 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,18 @@ 2009-11-09 Eric Blake + open: detect FreeBSD bug + * m4/open.m4 (gl_FUNC_OPEN): Also detect FreeBSD bug with slash on + symlink. + * doc/posix-functions/open.texi (open): Document the bug. + * doc/posix-functions/utimes.texi (utimes): Likewise. + * tests/test-open.h (test_open): Add parameters, and test symlink + handling. + * tests/test-open.c (main): Adjust caller. + * tests/test-fcntl-safer.c (main): Likewise. + * modules/open-tests (Depends-on): Add stdbool, symlink. + * modules/fcntl-safer-tests (Depends-on): Likewise. + * tests/test-openat.c (main): Add test-open tests. + stat: detect FreeBSD bug * m4/stat.m4 (gl_FUNC_STAT): Also detect FreeBSD bug with slash on symlink. diff --git a/doc/posix-functions/open.texi b/doc/posix-functions/open.texi index 6a919d815..933a24615 100644 --- a/doc/posix-functions/open.texi +++ b/doc/posix-functions/open.texi @@ -12,7 +12,7 @@ Portability problems fixed by the Gnulib module open: This function does not fail when the file name argument ends in a slash and (without the slash) names a nonexistent file or a file that is not a directory, on some platforms: -HP-UX 11.00, Solaris 9, Irix 5.3. +FreeBSD 7.2, HP-UX 11.00, Solaris 9, Irix 5.3. @item On Windows platforms (excluding Cygwin), this function does usually not recognize the @file{/dev/null} filename. diff --git a/doc/posix-functions/utimes.texi b/doc/posix-functions/utimes.texi index 390aa988a..cfd8fd315 100644 --- a/doc/posix-functions/utimes.texi +++ b/doc/posix-functions/utimes.texi @@ -17,7 +17,7 @@ This function is missing on some platforms: mingw, Interix 3.5, BeOS. @item On some platforms, this function mis-handles trailing slash: -Solaris 9. +FreeBSD 7.2, Solaris 9. @item This function cannot set full timestamp resolution. In particular, some platforms incorrectly round rather than truncate. Use diff --git a/m4/open.m4 b/m4/open.m4 index c0eb8e862..ba7d8761b 100644 --- a/m4/open.m4 +++ b/m4/open.m4 @@ -1,4 +1,4 @@ -# open.m4 serial 7 +# open.m4 serial 8 dnl Copyright (C) 2007-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, @@ -13,10 +13,15 @@ AC_DEFUN([gl_FUNC_OPEN], ;; *) dnl open("foo/") should not create a file when the file name has a - dnl trailing slash. + dnl trailing slash. FreeBSD only has the problem on symlinks. + AC_CHECK_FUNCS_ONCE([lstat]) 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. + if test $ac_cv_func_lstat = yes; then + touch conftest.tmp + ln -s conftest.tmp conftest.lnk + fi AC_TRY_RUN([ #include #if HAVE_UNISTD_H @@ -24,18 +29,22 @@ AC_DEFUN([gl_FUNC_OPEN], #endif int main () { +#if HAVE_LSTAT + if (open ("conftest.lnk/", O_RDONLY) != -1) return 2; +#endif return open ("conftest.sl/", O_CREAT, 0600) >= 0; }], [gl_cv_func_open_slash=yes], [gl_cv_func_open_slash=no], [ changequote(,)dnl case "$host_os" in + freebsd*) gl_cv_func_open_slash="guessing no" ;; solaris2.[0-9]*) gl_cv_func_open_slash="guessing no" ;; hpux*) gl_cv_func_open_slash="guessing no" ;; *) gl_cv_func_open_slash="guessing yes" ;; esac changequote([,])dnl ]) - rm -f conftest.sl + rm -f conftest.sl conftest.tmp conftest.lnk ]) case "$gl_cv_func_open_slash" in *no) diff --git a/modules/fcntl-safer-tests b/modules/fcntl-safer-tests index 622914268..3e8a2f6ba 100644 --- a/modules/fcntl-safer-tests +++ b/modules/fcntl-safer-tests @@ -3,6 +3,8 @@ tests/test-open.h tests/test-fcntl-safer.c Depends-on: +stdbool +symlink configure.ac: diff --git a/modules/open-tests b/modules/open-tests index 42aa93c02..16d4a99ec 100644 --- a/modules/open-tests +++ b/modules/open-tests @@ -3,10 +3,11 @@ tests/test-open.h tests/test-open.c Depends-on: +stdbool +symlink configure.ac: Makefile.am: TESTS += test-open check_PROGRAMS += test-open - diff --git a/tests/test-fcntl-safer.c b/tests/test-fcntl-safer.c index 33c7c2cd8..15f6e2ca6 100644 --- a/tests/test-fcntl-safer.c +++ b/tests/test-fcntl-safer.c @@ -20,6 +20,24 @@ #include "fcntl--.h" +#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) + #define BASE "test-fcntl-safer.t" #include "test-open.h" @@ -27,5 +45,5 @@ int main (void) { - return test_open (); + return test_open (open, true); } diff --git a/tests/test-open.c b/tests/test-open.c index 738934e12..37109a58a 100644 --- a/tests/test-open.c +++ b/tests/test-open.c @@ -20,6 +20,24 @@ #include +#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) + #define BASE "test-open.t" #include "test-open.h" @@ -27,5 +45,5 @@ int main (void) { - return test_open (); + return test_open (open, true); } diff --git a/tests/test-open.h b/tests/test-open.h index 4dba76708..9b4394582 100644 --- a/tests/test-open.h +++ b/tests/test-open.h @@ -16,29 +16,14 @@ /* Written by Bruno Haible , 2007. */ -/* Include and a form of first. */ - -#include -#include -#include -#include - -#define ASSERT(expr) \ - do \ - { \ - if (!(expr)) \ - { \ - fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ - fflush (stderr); \ - abort (); \ - } \ - } \ - while (0) - -/* Test fopen. Assumes BASE is defined. */ +/* This file is designed to test both open(n,buf[,mode]) and + openat(AT_FDCWD,n,buf[,mode]). FUNC is the function to test. + Assumes that BASE and ASSERT are already defined, and that + appropriate headers are already included. If PRINT, warn before + skipping symlink tests with status 77. */ static int -test_open (void) +test_open (int (*func) (char const *, int, ...), bool print) { int fd; /* Remove anything from prior partial run. */ @@ -46,40 +31,57 @@ test_open (void) /* Cannot create directory. */ errno = 0; - ASSERT (open ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) == -1); + ASSERT (func ("nonexist.ent/", O_CREAT | O_RDONLY, 0600) == -1); ASSERT (errno == ENOTDIR || errno == EISDIR || errno == ENOENT || errno == EINVAL); /* Create a regular file. */ - fd = open (BASE "file", O_CREAT | O_RDONLY, 0600); + fd = func (BASE "file", O_CREAT | O_RDONLY, 0600); ASSERT (0 <= fd); ASSERT (close (fd) == 0); /* Trailing slash handling. */ errno = 0; - ASSERT (open (BASE "file/", O_RDONLY) == -1); + ASSERT (func (BASE "file/", O_RDONLY) == -1); ASSERT (errno == ENOTDIR || errno == EISDIR || errno == EINVAL); /* Directories cannot be opened for writing. */ errno = 0; - ASSERT (open (".", O_WRONLY) == -1); + ASSERT (func (".", O_WRONLY) == -1); ASSERT (errno == EISDIR || errno == EACCES); /* /dev/null must exist, and be writable. */ - fd = open ("/dev/null", O_RDONLY); + fd = func ("/dev/null", O_RDONLY); ASSERT (0 <= fd); { char c; ASSERT (read (fd, &c, 1) == 0); } ASSERT (close (fd) == 0); - fd = open ("/dev/null", O_WRONLY); + fd = func ("/dev/null", O_WRONLY); ASSERT (0 <= fd); ASSERT (write (fd, "c", 1) == 1); ASSERT (close (fd) == 0); + /* Symlink handling, where supported. */ + if (symlink (BASE "file", BASE "link") != 0) + { + ASSERT (unlink (BASE "file") == 0); + if (print) + fputs ("skipping test: symlinks not supported on this file system\n", + stderr); + return 77; + } + errno = 0; + ASSERT (func (BASE "link/", O_RDONLY) == -1); + ASSERT (errno == ENOTDIR); + fd = func (BASE "link", O_RDONLY); + ASSERT (0 <= fd); + ASSERT (close (fd) == 0); + /* Cleanup. */ ASSERT (unlink (BASE "file") == 0); + ASSERT (unlink (BASE "link") == 0); return 0; } diff --git a/tests/test-openat.c b/tests/test-openat.c index 6e5c5199c..77185cc45 100644 --- a/tests/test-openat.c +++ b/tests/test-openat.c @@ -20,6 +20,9 @@ #include +#include +#include +#include #include #include #include @@ -28,20 +31,51 @@ do \ { \ if (!(expr)) \ - { \ - fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ - fflush (stderr); \ - abort (); \ - } \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ } \ while (0) +#define BASE "test-openat.t" + +#include "test-open.h" + +static int dfd = AT_FDCWD; + +/* Wrapper around openat to test open behavior. */ +static int +do_open (char const *name, int flags, ...) +{ + if (flags & O_CREAT) + { + mode_t mode = 0; + va_list arg; + va_start (arg, flags); + + /* We have to use PROMOTED_MODE_T instead of mode_t, otherwise GCC 4 + creates crashing code when 'mode_t' is smaller than 'int'. */ + mode = va_arg (arg, PROMOTED_MODE_T); + + va_end (arg); + return openat (dfd, name, flags, mode); + } + return openat (dfd, name, flags); +} + int main (void) { - /* FIXME - add more tests. For example, share /dev/null and - trailing slash tests with test-open, and do more checks for - proper fd handling. */ + int result; + + /* Basic checks. */ + result = test_open (do_open, false); + dfd = open (".", O_RDONLY); + ASSERT (0 <= dfd); + ASSERT (test_open (do_open, false) == result); + ASSERT (close (dfd) == 0); /* 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 @@ -49,12 +83,12 @@ main (void) ASSERT (close (STDIN_FILENO) == 0); ASSERT (openat (AT_FDCWD, ".", O_RDONLY) == STDIN_FILENO); { - int dfd = open (".", O_RDONLY); + 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; + return result; } -- 2.11.0