-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
According to Eric Blake on 11/7/2009 5:12 PM:
> Here's part of the difference - Solaris 9 mistakenly allows stat("file/")
> and stat("link-to-file/") to succeed, while FreeBSD only allowed the
> latter. Since the m4 test only covered the former, we weren't replacing
> stat on FreeBSD, even though we needed to.
Likewise for open (test-utimens passes once stat and open are fixed).
- --
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/
iEYEARECAAYFAkr2Uq8ACgkQ84KuGfSFAYAzLACePMGkOgHql40CGIvYuZjJ0HkX
KgoAoI1Dy60Oh/UBYZciy7BzoLjTE39k
=CETw
-----END PGP SIGNATURE-----
>From e67dbb6debf502cf746cc3981a0ba3c5cccd1fb5 Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Sat, 7 Nov 2009 21:34:32 -0700
Subject: [PATCH] open: detect FreeBSD bug
open("link-to-file/", O_RDONLY) mistakenly succeeds.
* m4/open.m4 (gl_FUNC_OPEN): Reject FreeBSD open.
* doc/posix-functions/open.texi (open): Document the bug.
* 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 <e...@byu.net>
---
ChangeLog | 11 ++++++++
doc/posix-functions/open.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 | 53 +++++++++++++++++++++++++++++++-------
9 files changed, 139 insertions(+), 45 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index ef20a64..c174a7d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
2009-11-07 Eric Blake <e...@byu.net>
+ open: detect FreeBSD bug
+ * m4/open.m4 (gl_FUNC_OPEN): Reject FreeBSD open.
+ * doc/posix-functions/open.texi (open): Document the bug.
+ * 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): Reject FreeBSD stat.
* doc/posix-functions/stat.texi (stat): Document the bug.
diff --git a/doc/posix-functions/open.texi b/doc/posix-functions/open.texi
index 6a919d8..933a246 100644
--- a/doc/posix-functions/open.texi
+++ b/doc/posix-functions/open.texi
@@ -12,7 +12,7 @@ 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/m4/open.m4 b/m4/open.m4
index c0eb8e8..ba7d876 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 <fcntl.h>
#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 6229142..3e8a2f6 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 42aa93c..16d4a99 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 33c7c2c..15f6e2c 100644
--- a/tests/test-fcntl-safer.c
+++ b/tests/test-fcntl-safer.c
@@ -20,6 +20,24 @@
#include "fcntl--.h"
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#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 738934e..37109a5 100644
--- a/tests/test-open.c
+++ b/tests/test-open.c
@@ -20,6 +20,24 @@
#include <fcntl.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#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 4dba767..9b43945 100644
--- a/tests/test-open.h
+++ b/tests/test-open.h
@@ -16,29 +16,14 @@
/* Written by Bruno Haible <br...@clisp.org>, 2007. */
-/* Include <config.h> and a form of <fcntl.h> first. */
-
-#include <errno.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <unistd.h>
-
-#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 6e5c519..b9b8932 100644
--- a/tests/test-openat.c
+++ b/tests/test-openat.c
@@ -20,6 +20,9 @@
#include <fcntl.h>
+#include <errno.h>
+#include <stdarg.h>
+#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
@@ -28,20 +31,50 @@
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, ...)
+{
+ mode_t mode = 0;
+ if (flags & O_CREAT)
+ {
+ 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);
+}
+
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 +82,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;
}
--
1.6.5.rc1
