# HG changeset patch # User Eric Blake # Date 1253142800 21600 # Node ID e8108d5c7ca74145955d40aef749a38fe3bef52c # Parent eacd308b94de8e98dc944869f8c6e1521a973d54 unlink: new module, for Solaris 9 bug unlink("file/") mistakenly succeeded. This patch favors, but does not enforce, GNU semantics that unlink("link-to-dir/") flat-out fails rather than attempting to unlink "dir". * modules/unlink: New file. * lib/unlink.c: Likewise. * m4/unlink.m4 (gl_FUNC_UNLINK): Likewise. * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witnesses. * modules/unistd (Makefile.am): Use them. * lib/unistd.in.h (stat): Declare replacement. * MODULES.html.sh (systems lacking POSIX:2008): Mention module. * doc/posix-functions/unlink.texi (unlink): Likewise. * modules/unlink-tests: New test. * tests/test-unlink.c: Likewise. Signed-off-by: Eric Blake diff --git a/ChangeLog b/ChangeLog --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,17 @@ 2009-09-19 Eric Blake + unlink: new module, for Solaris 9 bug + * modules/unlink: New file. + * lib/unlink.c: Likewise. + * m4/unlink.m4 (gl_FUNC_UNLINK): Likewise. + * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add witnesses. + * modules/unistd (Makefile.am): Use them. + * lib/unistd.in.h (stat): Declare replacement. + * MODULES.html.sh (systems lacking POSIX:2008): Mention module. + * doc/posix-functions/unlink.texi (unlink): Likewise. + * modules/unlink-tests: New test. + * tests/test-unlink.c: Likewise. + lstat: fix Solaris 9 bug * lib/lstat.c (lstat): Also check for trailing slash on non-symlink, non-directories. Use stat module to simplify logic. diff --git a/MODULES.html.sh b/MODULES.html.sh --- a/MODULES.html.sh +++ b/MODULES.html.sh @@ -2359,6 +2359,7 @@ func_module sys_wait func_module tsearch func_module unistd + func_module unlink func_module utime func_module vasnprintf-posix func_module vasprintf-posix diff --git a/doc/posix-functions/unlink.texi b/doc/posix-functions/unlink.texi --- a/doc/posix-functions/unlink.texi +++ b/doc/posix-functions/unlink.texi @@ -4,15 +4,28 @@ POSIX specification: @url{http://www.opengroup.org/onlinepubs/9699919799/functions/unlink.html} -Gnulib module: --- +Gnulib module: unlink Portability problems fixed by Gnulib: @itemize +@item +Some systems mistakenly succeed on @code{unlink("file/")}: +Solaris 9. @end itemize Portability problems not fixed by Gnulib: @itemize @item +Some systems allow a superuser to unlink directories, even though this +can cause file system corruption. The error given if a process is not +permitted to unlink directories varies across implementations; it is +not always the POSIX value of @code{EPERM}. Meanwhile, if a process +has the ability to unlink directories, POSIX requires that +@code{unlink("symlink-to-dir/")} remove @file{dir} and leave +@file{symlink-to-dir} dangling; this behavior is counter-intuitive. +The gnulib module unlinkdir can help determine whether code must be +cautious of unlinking directories. +@item Removing an open file is non-portable: On Unix this allows the programs that have the file already open to continue working with it; the file's storage is only freed when the no process has the file open any more. On Windows, diff --git a/lib/unistd.in.h b/lib/unistd.in.h --- a/lib/unistd.in.h +++ b/lib/unistd.in.h @@ -164,6 +164,21 @@ #endif +#if @GNULIB_UNLINK@ +# if @REPLACE_UNLINK@ +# undef unlink +# define unlink rpl_unlink +extern int unlink (char const *file); +# endif +#elif defined GNULIB_POSIXCHECK +# undef unlink +# define unlink(n) \ + (GL_LINK_WARNING ("unlink is not portable - " \ + "use gnulib module unlink for portability"), \ + unlink (n)) +#endif + + #if @GNULIB_UNLINKAT@ # if !@HAVE_UNLINKAT@ extern int unlinkat (int fd, char const *file, int flag); diff --git a/lib/unlink.c b/lib/unlink.c new file mode 100644 --- /dev/null +++ b/lib/unlink.c @@ -0,0 +1,85 @@ +/* Work around unlink bugs. + + 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 . */ + +#include + +#include + +#include +#include +#include +#include + +#undef unlink + +/* Remove file NAME. + Return 0 if successful, -1 if not. */ + +int +rpl_unlink (char const *name) +{ + /* Work around Solaris 9 bug where unlink("file/") succeeds. */ + size_t len = strlen (name); + int result = 0; + if (len && ISSLASH (name[len - 1])) + { + /* We can't unlink(2) something if it doesn't exist. If it does + exist, then it resolved to a directory, due to the trailing + slash, and POSIX requires that the unlink attempt to remove + that directory (which would leave the symlink dangling). + Unfortunately, Solaris 9 is one of the platforms where the + root user can unlink directories, and we don't want to + cripple this behavior on real directories, even if it is + seldom needed (at any rate, it's nicer to let coreutils' + unlink(1) give the correct errno for non-root users). But we + don't know whether name was an actual directory, or a symlink + to a directory; and due to the bug of ignoring trailing + slash, Solaris 9 would end up successfully unlinking the + symlink instead of the directory. Technically, we could use + realpath to find the canonical directory name to attempt + deletion on. But that is a lot of work for a corner case; so + we instead just use an lstat on the shortened name, and + reject symlinks with trailing slashes. The root user of + unlink(1) will just have to live with the rule that they + can't delete a directory via a symlink. */ + struct stat st; + result = lstat (name, &st); + if (result == 0) + { + /* Trailing NUL will overwrite the trailing slash. */ + char *short_name = malloc (len); + if (!short_name) + { + errno = EPERM; + return -1; + } + memcpy (short_name, name, len); + while (len && ISSLASH (short_name[len - 1])) + short_name[--len] = '\0'; + if (len && (lstat (short_name, &st) || S_ISLNK (st.st_mode))) + { + free (short_name); + errno = EPERM; + return -1; + } + free (short_name); + } + } + if (!result) + result = unlink (name); + return result; +} diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4 --- a/m4/unistd_h.m4 +++ b/m4/unistd_h.m4 @@ -1,4 +1,4 @@ -# unistd_h.m4 serial 27 +# unistd_h.m4 serial 28 dnl Copyright (C) 2006-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, @@ -61,6 +61,7 @@ GNULIB_SYMLINKAT=0; AC_SUBST([GNULIB_SYMLINKAT]) GNULIB_UNISTD_H_GETOPT=0; AC_SUBST([GNULIB_UNISTD_H_GETOPT]) GNULIB_UNISTD_H_SIGPIPE=0; AC_SUBST([GNULIB_UNISTD_H_SIGPIPE]) + GNULIB_UNLINK=0; AC_SUBST([GNULIB_UNLINK]) GNULIB_UNLINKAT=0; AC_SUBST([GNULIB_UNLINKAT]) GNULIB_WRITE=0; AC_SUBST([GNULIB_WRITE]) dnl Assume proper GNU behavior unless another module says otherwise. @@ -99,6 +100,7 @@ REPLACE_LINK=0; AC_SUBST([REPLACE_LINK]) REPLACE_LSEEK=0; AC_SUBST([REPLACE_LSEEK]) REPLACE_RMDIR=0; AC_SUBST([REPLACE_RMDIR]) + REPLACE_UNLINK=0; AC_SUBST([REPLACE_UNLINK]) REPLACE_WRITE=0; AC_SUBST([REPLACE_WRITE]) UNISTD_H_HAVE_WINSOCK2_H=0; AC_SUBST([UNISTD_H_HAVE_WINSOCK2_H]) UNISTD_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS=0; diff --git a/m4/unlink.m4 b/m4/unlink.m4 new file mode 100644 --- /dev/null +++ b/m4/unlink.m4 @@ -0,0 +1,27 @@ +# unlink.m4 serial 1 +dnl Copyright (C) 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, +dnl with or without modifications, as long as this notice is preserved. + +AC_DEFUN([gl_FUNC_UNLINK], +[ + AC_REQUIRE([gl_AC_DOS]) + AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) + dnl Detect Solaris 9 bug. + AC_CACHE_CHECK([whether unlink honors trailing slashes], + [gl_cv_func_unlink_works], + [touch conftest.file + AC_RUN_IFELSE( + [AC_LANG_PROGRAM( + [[#include + #include +]], [[return !unlink ("conftest.file/") || errno != ENOTDIR;]])], + [gl_cv_func_unlink_works=yes], [gl_cv_func_unlink_works=no], + [gl_cv_func_unlink_works="guessing no"]) + rm -f conftest.file]) + if test x"$gl_cv_func_unlink_works" != xyes; then + REPLACE_UNLINK=1 + AC_LIBOBJ([unlink]) + fi +]) diff --git a/modules/unistd b/modules/unistd --- a/modules/unistd +++ b/modules/unistd @@ -54,6 +54,7 @@ -e 's|@''GNULIB_SYMLINKAT''@|$(GNULIB_SYMLINKAT)|g' \ -e 's|@''GNULIB_UNISTD_H_GETOPT''@|$(GNULIB_UNISTD_H_GETOPT)|g' \ -e 's|@''GNULIB_UNISTD_H_SIGPIPE''@|$(GNULIB_UNISTD_H_SIGPIPE)|g' \ + -e 's|@''GNULIB_UNLINK''@|$(GNULIB_UNLINK)|g' \ -e 's|@''GNULIB_UNLINKAT''@|$(GNULIB_UNLINKAT)|g' \ -e 's|@''GNULIB_WRITE''@|$(GNULIB_WRITE)|g' \ -e 's|@''HAVE_DUP2''@|$(HAVE_DUP2)|g' \ @@ -91,6 +92,7 @@ -e 's|@''REPLACE_LINK''@|$(REPLACE_LINK)|g' \ -e 's|@''REPLACE_LSEEK''@|$(REPLACE_LSEEK)|g' \ -e 's|@''REPLACE_RMDIR''@|$(REPLACE_RMDIR)|g' \ + -e 's|@''REPLACE_UNLINK''@|$(REPLACE_UNLINK)|g' \ -e 's|@''REPLACE_WRITE''@|$(REPLACE_WRITE)|g' \ -e 's|@''UNISTD_H_HAVE_WINSOCK2_H''@|$(UNISTD_H_HAVE_WINSOCK2_H)|g' \ -e 's|@''UNISTD_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS''@|$(UNISTD_H_HAVE_WINSOCK2_H_AND_USE_SOCKETS)|g' \ diff --git a/modules/unlink b/modules/unlink new file mode 100644 --- /dev/null +++ b/modules/unlink @@ -0,0 +1,26 @@ +Description: +unlink(): remove a file. + +Files: +lib/unlink.c +m4/dos.m4 +m4/unlink.m4 + +Depends-on: +lstat +unistd + +configure.ac: +gl_FUNC_UNLINK +gl_UNISTD_MODULE_INDICATOR([unlink]) + +Makefile.am: + +Include: + + +License: +LGPL + +Maintainer: +Eric Blake diff --git a/modules/unlink-tests b/modules/unlink-tests new file mode 100644 --- /dev/null +++ b/modules/unlink-tests @@ -0,0 +1,12 @@ +Files: +tests/test-unlink.c + +Depends-on: +unlinkdir + +configure.ac: +AC_CHECK_FUNCS_ONCE([symlink]) + +Makefile.am: +TESTS += test-unlink +check_PROGRAMS += test-unlink diff --git a/tests/test-unlink.c b/tests/test-unlink.c new file mode 100644 --- /dev/null +++ b/tests/test-unlink.c @@ -0,0 +1,110 @@ +/* Tests of unlink. + 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 +#include +#include +#include + +#include "unlinkdir.h" + +#if !HAVE_SYMLINK +# define symlink(a,b) (-1) +#endif + +#define ASSERT(expr) \ + do \ + { \ + if (!(expr)) \ + { \ + fprintf (stderr, "%s:%d: assertion failed\n", __FILE__, __LINE__); \ + fflush (stderr); \ + abort (); \ + } \ + } \ + while (0) + +#define BASE "test-unlink.t" + +int +main () +{ + /* Remove any leftovers from a previous partial run. */ + ASSERT (system ("rm -rf " BASE "*") == 0); + + /* Setup. */ + ASSERT (mkdir (BASE "dir", 0700) == 0); + ASSERT (close (creat (BASE "dir/file", 0600)) == 0); + + /* Basic error conditions. */ + errno = 0; + ASSERT (unlink ("") == -1); + ASSERT (errno == ENOENT); + errno = 0; + ASSERT (unlink (BASE "nosuch") == -1); + ASSERT (errno == ENOENT); + errno = 0; + ASSERT (unlink (BASE "nosuch/") == -1); + ASSERT (errno == ENOENT); + /* Resulting errno after directories is rather varied across + implementations (EPERM, EINVAL, EACCES, EBUSY, EISDIR, ENOTSUP); + however, we must be careful to not attempt unlink on a directory + unless we know it must fail. */ + if (cannot_unlink_dir ()) + { + ASSERT (unlink (".") == -1); + ASSERT (unlink ("..") == -1); + ASSERT (unlink ("/") == -1); + ASSERT (unlink (BASE "dir") == -1); + ASSERT (mkdir (BASE "dir1", 0700) == 0); + ASSERT (unlink (BASE "dir1") == -1); + ASSERT (rmdir (BASE "dir1") == 0); + } + errno = 0; + ASSERT (unlink (BASE "dir/file/") == -1); + ASSERT (errno == ENOTDIR); + + /* Test symlink behavior. Specifying trailing slash will attempt + unlink of a directory, so only attempt it if we know it must + fail. */ + if (symlink (BASE "dir", BASE "link") != 0) + { + ASSERT (unlink (BASE "dir/file") == 0); + ASSERT (rmdir (BASE "dir") == 0); + fputs ("skipping test: symlinks not supported on this filesystem\n", + stderr); + return 77; + } + if (cannot_unlink_dir ()) + ASSERT (unlink (BASE "link/") == -1); + ASSERT (unlink (BASE "link") == 0); + ASSERT (symlink (BASE "dir/file", BASE "link") == 0); + /* Order here proves unlink of a symlink does not follow through to + the file. */ + ASSERT (unlink (BASE "link") == 0); + ASSERT (unlink (BASE "dir/file") == 0); + ASSERT (rmdir (BASE "dir") == 0); + + return 0; +}