-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 According to Eric Blake on 11/10/2009 6:51 AM: >> At least a couple more to go. I've confirmed that on FreeBSD: > > Also buggy: > > rm a > mkfifo b/ => mistakenly creates a
Furthermore, FreeBSD and OpenBSD (and probably NetBSD, but I haven't checked yet) fail to comply with POSIX in that mknod() fails with EPERM for non-root even when used to create a pipe. POSIX is explicit that mknod must succeed in that case. Both the BSD /sbin/mknod and coreutils' mknod.c work around this issue by calling mkfifo under the hood for 'mknod pipe p'. This was the patch that I've tested so far for symlink and mkfifoat, but the mkfifo portion still needs some work due to this latest wrinkle. I went ahead and pushed the symlink portion. I also discovered that readlink is broken on FreeBSD: $ touch a $ ln -s a b $ ln -s b c $ readlink c/ a => Oops - read the contents of b. - -- 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/ iEYEARECAAYFAkr6umEACgkQ84KuGfSFAYBmGwCfVFt6qFRBvsh7G8ah/Y5Dp6Ir vmsAoISWgugY8kyYoAfaiFnUd59XMGGo =2F10 -----END PGP SIGNATURE-----
>From 73af3ed572cc03ca130cf4cd0b530b0e4b6b77dc Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 10 Nov 2009 07:59:39 -0700 Subject: [PATCH 1/2] symlink: detect FreeBSD bug symlink(name,"dangling/") mistakenly created a symlink at the target of "dangling". * m4/symlink.m4 (gl_FUNC_SYMLINK): Also detect FreeBSD bug with slash on symlink. * doc/posix-functions/symlink.texi (symlink): Document the bug. * tests/test-symlink.h (test_symlink): Enhance test. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 8 ++++++++ doc/posix-functions/symlink.texi | 2 +- m4/symlink.m4 | 13 ++++++++----- tests/test-symlink.h | 28 +++++++++++++++++++++++----- 4 files changed, 40 insertions(+), 11 deletions(-) diff --git a/ChangeLog b/ChangeLog index 8863561..c8e1374 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,13 @@ 2009-11-10 Eric Blake <e...@byu.net> + symlink: detect FreeBSD bug + * m4/symlink.m4 (gl_FUNC_SYMLINK): Also detect FreeBSD bug with + slash on symlink. + * doc/posix-functions/symlink.texi (symlink): Document the bug. + * tests/test-symlink.h (test_symlink): Enhance test. + +2009-11-10 Eric Blake <e...@byu.net> + link: detect FreeBSD bug * m4/link.m4 (gl_FUNC_LINK): Also detect FreeBSD bug with slash on symlink. diff --git a/doc/posix-functions/symlink.texi b/doc/posix-functions/symlink.texi index de7c0aa..b3d740f 100644 --- a/doc/posix-functions/symlink.texi +++ b/doc/posix-functions/symlink.texi @@ -11,7 +11,7 @@ symlink @item On some systems, @code{symlink(value,"name/")} mistakenly creates a symlink: -Solaris 9 +FreeBSD 7.2, Solaris 9 @item This function is missing on some platforms; however, the replacement always fails with @code{EPERM}: diff --git a/m4/symlink.m4 b/m4/symlink.m4 index abea045..010d4b8 100644 --- a/m4/symlink.m4 +++ b/m4/symlink.m4 @@ -1,4 +1,4 @@ -# serial 1 +# serial 2 # See if we need to provide symlink replacement. dnl Copyright (C) 2009 Free Software Foundation, Inc. @@ -13,8 +13,8 @@ AC_DEFUN([gl_FUNC_SYMLINK], AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) AC_CHECK_FUNCS_ONCE([symlink]) dnl The best we can do on mingw is provide a dummy that always fails, so - dnl that compilation can proceed with fewer ifdefs. On Solaris 9, we - dnl want to fix a bug with trailing slash handling. + dnl that compilation can proceed with fewer ifdefs. On Solaris 9 and + dnl FreeBSD 7.2, we want to fix a bug with trailing slash handling. if test $ac_cv_func_symlink = no; then HAVE_SYMLINK=0 AC_LIBOBJ([symlink]) @@ -24,9 +24,12 @@ AC_DEFUN([gl_FUNC_SYMLINK], [AC_RUN_IFELSE( [AC_LANG_PROGRAM( [[#include <unistd.h> -]], [[return !symlink ("a", "conftest.link/");]])], +]], [[if (!symlink ("a", "conftest.link/")) return 1; + if (symlink ("conftest.f", "conftest.lnk2")) return 2; + if (!symlink ("a", "conftest.lnk2/")) return 3;]])], [gl_cv_func_symlink_works=yes], [gl_cv_func_symlink_works=no], - [gl_cv_func_symlink_works="guessing no"])]) + [gl_cv_func_symlink_works="guessing no"]) + rm -f conftest.f conftest.link conftest.lnk2]) if test "$gl_cv_func_symlink_works" != yes; then REPLACE_SYMLINK=1 AC_LIBOBJ([symlink]) diff --git a/tests/test-symlink.h b/tests/test-symlink.h index d009a80..c083c6c 100644 --- a/tests/test-symlink.h +++ b/tests/test-symlink.h @@ -35,11 +35,18 @@ test_symlink (int (*func) (char const *, char const *), bool print) /* Some systems allow the creation of 0-length symlinks as a synonym for "."; but most reject it. */ - errno = 0; - if (func ("", BASE "link2") == -1) - ASSERT (errno == ENOENT || errno == EINVAL); - else - ASSERT (unlink (BASE "link2") == 0); + { + int status; + errno = 0; + status = func ("", BASE "link2"); + if (status == -1) + ASSERT (errno == ENOENT || errno == EINVAL); + else + { + ASSERT (status == 0); + ASSERT (unlink (BASE "link2") == 0); + } + } /* Sanity checks of failures. */ errno = 0; @@ -69,6 +76,17 @@ test_symlink (int (*func) (char const *, char const *), bool print) ASSERT (func ("nowhere", BASE "file/") == -1); ASSERT (errno == EEXIST || errno == ENOTDIR || errno == ENOENT); + /* Trailing slash must always be rejected. */ + ASSERT (unlink (BASE "link1") == 0); + ASSERT (func (BASE "link2", BASE "link1") == 0); + errno = 0; + ASSERT (func (BASE "nowhere", BASE "link1/") == -1); + ASSERT (errno == EEXIST || errno == ENOTDIR || errno == ENOENT); + errno = 0; + ASSERT (unlink (BASE "link2") == -1); + ASSERT (errno == ENOENT); + + /* Cleanup. */ ASSERT (rmdir (BASE "dir") == 0); ASSERT (unlink (BASE "file") == 0); ASSERT (unlink (BASE "link1") == 0); -- 1.6.5.rc1 >From 9550e46ea46d813c1d360db5d818720b98dcf85a Mon Sep 17 00:00:00 2001 From: Eric Blake <e...@byu.net> Date: Tue, 10 Nov 2009 08:58:18 -0700 Subject: [PATCH 2/2] mkfifoat: detect Solaris 9 and FreeBSD bug On Solaris 9, mkfifoat(fd,"missing/",mode) mistakenly created "missing". On FreeBSD 7.2, mkfifoat(fd,"dangling/",mode) mistakenly created a fifo at the target of "dangling". * m4/mkfifoat.m4 (gl_FUNC_MKFIFOAT): Detect bugs with trailing slash handling. * lib/mkfifoat.c (mkfifoat): Work around the bug. * modules/mkfifoat (Depends-on): Add lstat. * doc/posix-functions/mkfifo.texi (mkfifo): Document the bug. * doc/posix-functions/mknod.texi (mknod): Likewise. * tests/test-mkfifoat.c (main): Enhance test. Signed-off-by: Eric Blake <e...@byu.net> --- ChangeLog | 11 +++++++ doc/posix-functions/mkfifo.texi | 5 ++- doc/posix-functions/mknod.texi | 5 ++- lib/mkfifoat.c | 59 +++++++++++++++++++++++++++++++++++++- m4/mkfifoat.m4 | 28 ++++++++++++++++++- modules/mkfifoat | 1 + tests/test-mkfifoat.c | 57 +++++++++++++++++++++++++------------ 7 files changed, 143 insertions(+), 23 deletions(-) diff --git a/ChangeLog b/ChangeLog index c8e1374..08d6bcf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,16 @@ 2009-11-10 Eric Blake <e...@byu.net> + mkfifoat: detect Solaris 9 and FreeBSD bug + * m4/mkfifoat.m4 (gl_FUNC_MKFIFOAT): Detect bugs with trailing + slash handling. + * lib/mkfifoat.c (mkfifoat): Work around the bug. + * modules/mkfifoat (Depends-on): Add lstat. + * doc/posix-functions/mkfifo.texi (mkfifo): Document the bug. + * doc/posix-functions/mknod.texi (mknod): Likewise. + * tests/test-mkfifoat.c (main): Enhance test. + +2009-11-10 Eric Blake <e...@byu.net> + symlink: detect FreeBSD bug * m4/symlink.m4 (gl_FUNC_SYMLINK): Also detect FreeBSD bug with slash on symlink. diff --git a/doc/posix-functions/mkfifo.texi b/doc/posix-functions/mkfifo.texi index 926f57d..c1792be 100644 --- a/doc/posix-functions/mkfifo.texi +++ b/doc/posix-functions/mkfifo.texi @@ -10,9 +10,12 @@ mkfifo @itemize @end itemize -Portability problems not fixed by Gnulib: +Portability problems not fixed by Gnulib (but see the mkfifoat module): @itemize @item +This function mishandles trailing slash on some platforms: +FreeBSD 7.2, Solaris 9. +...@item This function is missing on some platforms: mingw. @end itemize diff --git a/doc/posix-functions/mknod.texi b/doc/posix-functions/mknod.texi index 2853416..694acac 100644 --- a/doc/posix-functions/mknod.texi +++ b/doc/posix-functions/mknod.texi @@ -10,9 +10,12 @@ mknod @itemize @end itemize -Portability problems not fixed by Gnulib: +Portability problems not fixed by Gnulib (but see the mkfifoat module): @itemize @item +This function mishandles trailing slash on some platforms: +FreeBSD 7.2, Solaris 9. +...@item This function is missing on some platforms: mingw. @end itemize diff --git a/lib/mkfifoat.c b/lib/mkfifoat.c index 29fc070..89376f9 100644 --- a/lib/mkfifoat.c +++ b/lib/mkfifoat.c @@ -43,7 +43,7 @@ int mkfifoat (int fd _UNUSED_PARAMETER_, char const *path _UNUSED_PARAMETER_, - mode_t mode _UNUSED_PARAMETER_) + mode_t mode _UNUSED_PARAMETER_) { errno = ENOSYS; return -1; @@ -51,7 +51,7 @@ mkfifoat (int fd _UNUSED_PARAMETER_, char const *path _UNUSED_PARAMETER_, int mknodat (int fd _UNUSED_PARAMETER_, char const *path _UNUSED_PARAMETER_, - mode_t mode _UNUSED_PARAMETER_, dev_t dev _UNUSED_PARAMETER_) + mode_t mode _UNUSED_PARAMETER_, dev_t dev _UNUSED_PARAMETER_) { errno = ENOSYS; return -1; @@ -59,6 +59,61 @@ mknodat (int fd _UNUSED_PARAMETER_, char const *path _UNUSED_PARAMETER_, #else /* HAVE_MKFIFO */ +# if MKFIFO_TRAILING_SLASH_BUG +# include <errno.h> +# include "openat.h" + +/* If mkfifo and mknod don't reject trailing slash, then we wrap + mkfifoat and mknodat to detect that situation. */ + +static int local_mkfifoat (int, char const *, mode_t); +static int local_mknodat (int, char const *, mode_t, dev_t); + +/* Create a fifo NAME relative to FD with permissions in MODE, and + work around trailing slash bugs. */ +int +mkfifoat (int fd, char const *name, mode_t mode) +{ + size_t len = strlen (name); + if (len && name[len - 1] == '/') + { + struct stat st; + if (lstatat (fd, name, &st) == 0) + errno = EEXIST; + return -1; + } + return local_mkfifoat (fd, name, mode); +} + +/* Create an inode entry NAME relative to FD with permissions in MODE + and device type DEV. If used portably (that is, if MODE specifies + a fifo and DEV is 0), work around trailing slash bugs. */ +int +mknodat (int fd, char const *name, mode_t mode, dev_t dev) +{ + /* Only reject trailing slash when mknodat is being used to create a + FIFO; all other uses are implementation-defined, and some + implementations might allow the superuser to create a directory + using mknod and a trailing slash. */ + if (!dev && S_ISFIFO (mode)) + { + size_t len = strlen (name); + if (len && name[len - 1] == '/') + { + struct stat st; + if (lstatat (fd, name, &st) == 0) + errno = EEXIST; + return -1; + } + } + return local_mknodat (fd, name, mode, dev); +} + +# define mkfifoat local_mkfifoat +# define mknodat local_mknodat + +# endif /* MKFIFO_TRAILING_SLASH_BUG */ + /* Create a named fifo FILE relative to directory FD, with access permissions in MODE. If possible, do it without changing the working directory. Otherwise, resort to using save_cwd/fchdir, diff --git a/m4/mkfifoat.m4 b/m4/mkfifoat.m4 index 99e2336..5d28b0b 100644 --- a/m4/mkfifoat.m4 +++ b/m4/mkfifoat.m4 @@ -1,4 +1,4 @@ -# serial 1 +# serial 2 # See if we need to provide mkfifoat/mknodat replacement. dnl Copyright (C) 2009 Free Software Foundation, Inc. @@ -20,4 +20,30 @@ AC_DEFUN([gl_FUNC_MKFIFOAT], HAVE_MKNODAT=0 AC_LIBOBJ([mkfifoat]) fi + if test $ac_cv_func_mkfifo = yes; then + dnl Check for Solaris 9 and FreeBSD bug with trailing slash. + dnl No known system has both mkfifoat and trailing slash bug. + AC_CHECK_FUNCS_ONCE([lstat]) + AC_CACHE_CHECK([whether mkfifo rejects trailing slashes], + [gl_cv_func_mkfifo_works], + [# Assume that if we have lstat, we can also check symlinks. + if test $ac_cv_func_lstat = yes; then + ln -s conftest.tmp conftest.lnk + fi + AC_RUN_IFELSE( + [AC_LANG_PROGRAM( + [[#include <sys/stat.h> +]], [[if (!mkfifo ("conftest.tmp/", 0600)) return 1; +#if HAVE_LSTAT + if (!mkfifo ("conftest.lnk/", 0600)) return 2; +#endif + ]])], + [gl_cv_func_mkfifo_works=yes], [gl_cv_func_mkfifo_works=no], + [gl_cv_func_mkfifo_works="guessing no"]) + rm -f conftest.tmp conftest.lnk]) + if test "$gl_cv_func_mkfifo_works" != yes; then + AC_DEFINE([MKFIFO_TRAILING_SLASH_BUG], [1], [Define to 1 if mkfifo + and mknod do not reject trailing slash]) + fi + fi ]) diff --git a/modules/mkfifoat b/modules/mkfifoat index db2f0da..10fdecf 100644 --- a/modules/mkfifoat +++ b/modules/mkfifoat @@ -8,6 +8,7 @@ m4/mkfifoat.m4 Depends-on: extensions fcntl-h +lstat openat sys_stat diff --git a/tests/test-mkfifoat.c b/tests/test-mkfifoat.c index 2992ba2..ea26f17 100644 --- a/tests/test-mkfifoat.c +++ b/tests/test-mkfifoat.c @@ -31,14 +31,16 @@ 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-mkfifoat.t" + typedef int (*test_func) (int, char const *, mode_t); /* Wrapper to make testing mknodat easier. */ @@ -54,7 +56,6 @@ main (void) { int i; test_func funcs[2] = { mkfifoat, test_mknodat }; - const char *fifo = "test-mkfifoat.fifo"; /* Create handle for future use. */ int dfd = openat (AT_FDCWD, ".", O_RDONLY); @@ -65,8 +66,8 @@ main (void) return 77; #endif - /* Clean up anything from previous incomplete test. */ - remove (fifo); + /* Remove any leftovers from a previous partial run. */ + ASSERT (system ("rm -rf " BASE "*") == 0); /* Test both functions. */ for (i = 0; i < 2; i++) @@ -90,35 +91,55 @@ main (void) ASSERT (errno == EEXIST || errno == EINVAL); /* Create fifo while cwd is '.', then stat it from '..'. */ - ASSERT (func (AT_FDCWD, fifo, 0600) == 0); + ASSERT (func (AT_FDCWD, BASE "fifo", 0600) == 0); errno = 0; - ASSERT (func (dfd, fifo, 0600) == -1); + ASSERT (func (dfd, BASE "fifo", 0600) == -1); ASSERT (errno == EEXIST); ASSERT (chdir ("..") == 0); errno = 0; - ASSERT (fstatat (AT_FDCWD, fifo, &st, 0) == -1); + ASSERT (fstatat (AT_FDCWD, BASE "fifo", &st, 0) == -1); ASSERT (errno == ENOENT); memset (&st, 0, sizeof st); - ASSERT (fstatat (dfd, fifo, &st, 0) == 0); + ASSERT (fstatat (dfd, BASE "fifo", &st, 0) == 0); ASSERT (S_ISFIFO (st.st_mode)); - ASSERT (unlinkat (dfd, fifo, 0) == 0); + ASSERT (unlinkat (dfd, BASE "fifo", 0) == 0); /* Create fifo while cwd is '..', then stat it from '.'. */ - ASSERT (func (dfd, fifo, 0600) == 0); + ASSERT (func (dfd, BASE "fifo", 0600) == 0); ASSERT (fchdir (dfd) == 0); errno = 0; - ASSERT (func (AT_FDCWD, fifo, 0600) == -1); + ASSERT (func (AT_FDCWD, BASE "fifo", 0600) == -1); ASSERT (errno == EEXIST); memset (&st, 0, sizeof st); - ASSERT (fstatat (AT_FDCWD, fifo, &st, AT_SYMLINK_NOFOLLOW) == 0); + ASSERT (fstatat (AT_FDCWD, BASE "fifo", &st, AT_SYMLINK_NOFOLLOW) == 0); ASSERT (S_ISFIFO (st.st_mode)); memset (&st, 0, sizeof st); - ASSERT (fstatat (dfd, fifo, &st, AT_SYMLINK_NOFOLLOW) == 0); + ASSERT (fstatat (dfd, BASE "fifo", &st, AT_SYMLINK_NOFOLLOW) == 0); ASSERT (S_ISFIFO (st.st_mode)); - ASSERT (unlink (fifo) == 0); + ASSERT (unlink (BASE "fifo") == 0); } ASSERT (close (dfd) == 0); + /* Test trailing slash behavior. */ + if (symlink (BASE "fifo", BASE "link")) + { + fputs ("skipping test: symlinks not supported on this file system\n", + stderr); + return 77; + } + for (i = 0; i < 2; i++) + { + test_func func = funcs[i]; + errno = 0; + ASSERT (func (AT_FDCWD, BASE "fifo/", 0600) == -1); + ASSERT (errno == EINVAL || errno == ENOENT || errno == ENOTDIR); + errno = 0; + ASSERT (func (AT_FDCWD, BASE "link/", 0600) == -1); + ASSERT (errno == EINVAL || errno == ENOENT || errno == ENOTDIR + || errno == EEXIST); + } + ASSERT (unlink (BASE "link") == 0); + return 0; } -- 1.6.5.rc1