chown: work around OpenBSD bug
authorEric Blake <ebb9@byu.net>
Mon, 16 Nov 2009 21:35:41 +0000 (14:35 -0700)
committerEric Blake <ebb9@byu.net>
Wed, 18 Nov 2009 05:30:00 +0000 (22:30 -0700)
chown(name,geteuid(),-1) failed to update the change time if
name was already owned by the current effective user.  Work
around it by using chmod, which does not have this bug.

Unfortunately, lchown has the same bug, but OpenBSD 4.0 lacks
lchmod and lutimes, so there is no way to affect ctime without
unlinking and recreating the symlink, which is too dangerous.

* lib/chown.c (rpl_chown): Work around the bug.
* lib/lchown.c (rpl_lchown): Attempt to do likewise.
* m4/chown.m4 (gl_FUNC_CHOWN): Test for ctime bug.
* m4/lchown.m4 (gl_FUNC_LCHOWN): Check for lchmod.
* modules/chown (Depends-on): Add stdbool.
* modules/lchown (Depends-on): Likewise.
* doc/posix-functions/chown.texi (chown): Document the bug.
* doc/posix-functions/lchown.texi (lchown): Likewise.
* tests/test-lchown.h (test_chown): Relax test.

Signed-off-by: Eric Blake <ebb9@byu.net>
ChangeLog
doc/posix-functions/chown.texi
doc/posix-functions/lchown.texi
lib/chown.c
lib/lchown.c
m4/chown.m4
m4/lchown.m4
modules/chown
modules/lchown
tests/test-lchown.h

index 64b11d23e15b6b4a8c0d6cd0b62270b3ed20941b..599a94413fa6e8e068301dcb4add0e8d53d3709d 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,16 @@
 2009-11-17  Eric Blake  <ebb9@byu.net>
 
+       chown: work around OpenBSD bug
+       * lib/chown.c (rpl_chown): Work around the bug.
+       * lib/lchown.c (rpl_lchown): Attempt to do likewise.
+       * m4/chown.m4 (gl_FUNC_CHOWN): Test for ctime bug.
+       * m4/lchown.m4 (gl_FUNC_LCHOWN): Check for lchmod.
+       * modules/chown (Depends-on): Add stdbool.
+       * modules/lchown (Depends-on): Likewise.
+       * doc/posix-functions/chown.texi (chown): Document the bug.
+       * doc/posix-functions/lchown.texi (lchown): Likewise.
+       * tests/test-lchown.h (test_chown): Relax test.
+
        mkstemp: avoid conflict with C++ keyword template
        * lib/mkdtemp.c (mkdtemp): Change spelling of template.
        * lib/mkostemp.c (mkostemp): Likewise.
index e5a80d6a1fe5b0b08b1d6c04e04b2b3a8c47505f..88e25cd21bc9a946dbb74f18f809367cfe5a654a 100644 (file)
@@ -13,6 +13,10 @@ Some platforms fail to detect trailing slash on non-directories, as in
 @code{chown("link-to-file/",uid,gid)}:
 FreeBSD 7.2, Solaris 9.
 @item
+Some platforms fail to update the change time when at least one
+argument was not -1, but no ownership changes resulted:
+OpenBSD 4.0.
+@item
 When passed an argument of -1, some implementations really set the owner
 user/group id of the file to this value, rather than leaving that id of the
 file alone.
index 0686bf3917adf2a4bfe23e28d86f4b15376a86d7..595a05ce9527f73088f085eee7f15ec32f2181c3 100644 (file)
@@ -13,6 +13,11 @@ Some platforms fail to detect trailing slash on non-directories, as in
 @code{lchown("link-to-file/",uid,gid)}:
 FreeBSD 7.2, Solaris 9.
 @item
+Some platforms fail to update the change time when at least one
+argument was not -1, but no ownership changes resulted.  However,
+without @code{lchmod}, the replacement only fixes this for non-symlinks:
+OpenBSD 4.0.
+@item
 This function is missing on some platforms; however, the replacement
 fails on symlinks if @code{chown} is supported, and fails altogether
 with @code{ENOSYS} otherwise:
index edbccc6d6efde9904292f2d771f5ac6eed5d1700..66bb0c8fe1a5a46de5795314c0106bceb28e34d9 100644 (file)
@@ -59,20 +59,29 @@ chown (const char *file _UNUSED_PARAMETER_, uid_t uid _UNUSED_PARAMETER_,
 int
 rpl_chown (const char *file, uid_t uid, gid_t gid)
 {
+  struct stat st;
+  bool stat_valid = false;
+  int result;
+
+# if CHOWN_CHANGE_TIME_BUG
+  if (gid != (gid_t) -1 || uid != (uid_t) -1)
+    {
+      if (stat (file, &st))
+        return -1;
+      stat_valid = true;
+    }
+# endif
+
 # if CHOWN_FAILS_TO_HONOR_ID_OF_NEGATIVE_ONE
   if (gid == (gid_t) -1 || uid == (uid_t) -1)
     {
-      struct stat file_stats;
-
       /* Stat file to get id(s) that should remain unchanged.  */
-      if (stat (file, &file_stats))
+      if (!stat_valid && stat (file, &st))
         return -1;
-
       if (gid == (gid_t) -1)
-        gid = file_stats.st_gid;
-
+        gid = st.st_gid;
       if (uid == (uid_t) -1)
-        uid = file_stats.st_uid;
+        uid = st.st_uid;
     }
 # endif
 
@@ -89,15 +98,18 @@ rpl_chown (const char *file, uid_t uid, gid_t gid)
         || (errno == EACCES
             && 0 <= (fd = open (file, O_WRONLY | open_flags))))
       {
-        int result = fchown (fd, uid, gid);
-        int saved_errno = errno;
+        int saved_errno;
+        bool fchown_socket_failure;
 
-        /* POSIX says fchown can fail with errno == EINVAL on sockets,
-           so fall back on chown in that case.  */
-        struct stat sb;
-        bool fchown_socket_failure =
+        result = fchown (fd, uid, gid);
+        saved_errno = errno;
+
+        /* POSIX says fchown can fail with errno == EINVAL on sockets
+           and pipes, so fall back on chown in that case.  */
+        fchown_socket_failure =
           (result != 0 && saved_errno == EINVAL
-           && fstat (fd, &sb) == 0 && S_ISFIFO (sb.st_mode));
+           && fstat (fd, &st) == 0
+           && (S_ISFIFO (st.st_mode) || S_ISSOCK (st.st_mode)));
 
         close (fd);
 
@@ -113,15 +125,32 @@ rpl_chown (const char *file, uid_t uid, gid_t gid)
 # endif
 
 # if CHOWN_TRAILING_SLASH_BUG
-  {
-    size_t len = strlen (file);
-    struct stat st;
-    if (len && file[len - 1] == '/' && stat (file, &st))
-      return -1;
-  }
+  if (!stat_valid)
+    {
+      size_t len = strlen (file);
+      if (len && file[len - 1] == '/' && stat (file, &st))
+        return -1;
+    }
+# endif
+
+  result = chown (file, uid, gid);
+
+# if CHOWN_CHANGE_TIME_BUG
+  if (result == 0 && stat_valid
+      && (uid == st.st_uid || uid == (uid_t) -1)
+      && (gid == st.st_gid || gid == (gid_t) -1))
+    {
+      /* No change in ownership, but at least one argument was not -1,
+         so we are required to update ctime.  Since chown succeeded,
+         we assume that chmod will do likewise.  Fortunately, on all
+         known systems where a 'no-op' chown skips the ctime update, a
+         'no-op' chmod still does the trick.  */
+      result = chmod (file, st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO
+                                          | S_ISUID | S_ISGID | S_ISVTX));
+    }
 # endif
 
-  return chown (file, uid, gid);
+  return result;
 }
 
 #endif /* HAVE_CHOWN */
index 265c2f72ec1d442fabf862840b8daf4fa421e926..19eb9c6c57ef4d6fb30464b997cd0bec4774d058 100644 (file)
@@ -23,6 +23,7 @@
 #include <unistd.h>
 
 #include <errno.h>
+#include <stdbool.h>
 #include <string.h>
 #include <sys/stat.h>
 
@@ -69,10 +70,47 @@ lchown (const char *file, uid_t uid, gid_t gid)
 int
 rpl_lchown (const char *file, uid_t uid, gid_t gid)
 {
-  size_t len = strlen (file);
-  if (len && file[len - 1] == '/')
-    return chown (file, uid, gid);
-  return lchown (file, uid, gid);
+  struct stat st;
+  bool stat_valid = false;
+  int result;
+
+# if CHOWN_CHANGE_TIME_BUG
+  if (gid != (gid_t) -1 || uid != (uid_t) -1)
+    {
+      if (lstat (file, &st))
+        return -1;
+      stat_valid = true;
+      if (!S_ISLNK (st.st_mode))
+        return chown (file, uid, gid);
+    }
+# endif
+
+# if CHOWN_TRAILING_SLASH_BUG
+  if (!stat_valid)
+    {
+      size_t len = strlen (file);
+      if (len && file[len - 1] == '/')
+        return chown (file, uid, gid);
+    }
+# endif
+
+  result = lchown (file, uid, gid);
+
+# if CHOWN_CHANGE_TIME_BUG && HAVE_LCHMOD
+  if (result == 0 && stat_valid
+      && (uid == st.st_uid || uid == (uid_t) -1)
+      && (gid == st.st_gid || gid == (gid_t) -1))
+    {
+      /* No change in ownership, but at least one argument was not -1,
+         so we are required to update ctime.  Since lchown succeeded,
+         we assume that lchmod will do likewise.  But if the system
+         lacks lchmod and lutimes, we are out of luck.  Oh well.  */
+      result = lchmod (file, st.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO
+                                           | S_ISUID | S_ISGID | S_ISVTX));
+    }
+# endif
+
+  return result;
 }
 
 #endif /* HAVE_LCHOWN */
index 5bedfa192bafb5dc42137e7f7b3d653eabc4f502..0dced4bce06a348ce963ca117a94a789f8f40e8f 100644 (file)
@@ -1,4 +1,4 @@
-# serial 21
+# serial 22
 # Determine whether we need the chown wrapper.
 
 dnl Copyright (C) 1997-2001, 2003-2005, 2007, 2009
@@ -22,20 +22,27 @@ AC_DEFUN_ONCE([gl_FUNC_CHOWN],
   AC_REQUIRE([gl_FUNC_CHOWN_FOLLOWS_SYMLINK])
   AC_CHECK_FUNCS_ONCE([chown fchown])
 
+  dnl mingw lacks chown altogether.
   if test $ac_cv_func_chown = no; then
     HAVE_CHOWN=0
     AC_LIBOBJ([chown])
   else
+    dnl Some old systems treated chown like lchown.
     if test $gl_cv_func_chown_follows_symlink = no; then
       REPLACE_CHOWN=1
       AC_LIBOBJ([chown])
     fi
+
+    dnl Some old systems tried to use uid/gid -1 literally.
     if test $ac_cv_func_chown_works = no; then
       AC_DEFINE([CHOWN_FAILS_TO_HONOR_ID_OF_NEGATIVE_ONE], [1],
         [Define if chown is not POSIX compliant regarding IDs of -1.])
       REPLACE_CHOWN=1
       AC_LIBOBJ([chown])
     fi
+
+    dnl Solaris 9 ignores trailing slash.
+    dnl FreeBSD 7.2 mishandles trailing slash on symlinks.
     AC_CACHE_CHECK([whether chown honors trailing slash],
       [gl_cv_func_chown_slash_works],
       [touch conftest.file && rm -f conftest.link
@@ -52,10 +59,39 @@ AC_DEFUN_ONCE([gl_FUNC_CHOWN],
       rm -f conftest.link conftest.file])
     if test "$gl_cv_func_chown_slash_works" != yes; then
       AC_DEFINE([CHOWN_TRAILING_SLASH_BUG], [1],
-        [Define if chown mishandles trailing slash.])
+        [Define to 1 if chown mishandles trailing slash.])
       REPLACE_CHOWN=1
       AC_LIBOBJ([chown])
     fi
+
+    dnl OpenBSD fails to update ctime if ownership does not change.
+    AC_CACHE_CHECK([whether chown always updates ctime],
+      [gl_cv_func_chown_ctime_works],
+      [AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#include <unistd.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+]], [[    struct stat st1, st2;
+          if (close (creat ("conftest.file", 0600))) return 1;
+          if (stat ("conftest.file", &st1)) return 2;
+          sleep (1);
+          if (chown ("conftest.file", st1.st_uid, st1.st_gid)) return 3;
+          if (stat ("conftest.file", &st2)) return 4;
+          if (st2.st_ctime <= st1.st_ctime) return 5;
+        ]])],
+        [gl_cv_func_chown_ctime_works=yes],
+        [gl_cv_func_chown_ctime_works=no],
+        [gl_cv_func_chown_ctime_works="guessing no"])
+      rm -f conftest.file])
+    if test "$gl_cv_func_chown_ctime_works" != yes; then
+      AC_DEFINE([CHOWN_CHANGE_TIME_BUG], [1], [Define to 1 if chown fails
+        to change ctime when at least one argument was not -1.])
+      REPLACE_CHOWN=1
+      AC_LIBOBJ([chown])
+    fi
+
     if test $REPLACE_CHOWN = 1 && test $ac_cv_func_fchown = no; then
       AC_LIBOBJ([fchown-stub])
     fi
index e40c4376262f7ae8509415cf1b03986436de01a5..f0d67fe806cc6bc9e516d44d00550a3c4f789f67 100644 (file)
@@ -1,4 +1,4 @@
-# serial 14
+# serial 15
 # Determine whether we need the lchown wrapper.
 
 dnl Copyright (C) 1998, 2001, 2003-2007, 2009 Free Software
@@ -9,18 +9,20 @@ dnl gives unlimited permission to copy and/or distribute it,
 dnl with or without modifications, as long as this notice is preserved.
 
 dnl From Jim Meyering.
-dnl Provide lchown on systems that lack it, and work around trailing
-dnl slash bugs on systems that have it.
+dnl Provide lchown on systems that lack it, and work around bugs
+dnl on systems that have it.
 
 AC_DEFUN([gl_FUNC_LCHOWN],
 [
   AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
   AC_REQUIRE([gl_FUNC_CHOWN])
+  AC_CHECK_FUNCS_ONCE([lchmod])
   AC_REPLACE_FUNCS([lchown])
   if test $ac_cv_func_lchown = no; then
     HAVE_LCHOWN=0
-  elif test "$gl_cv_func_chown_slash_works" != yes; then
-    dnl Trailing slash bugs in chown also occur in lchown.
+  elif test "$gl_cv_func_chown_slash_works" != yes \
+      || test "$gl_cv_func_chown_ctime_works" != yes; then
+    dnl Trailing slash and ctime bugs in chown also occur in lchown.
     AC_LIBOBJ([lchown])
     REPLACE_LCHOWN=1
   fi
index 88d0cd458fe35b7d2bd4a55c55af0a70843ad4b4..4c296ac2cc2ada9b9753eab503e8ff074fb8e642 100644 (file)
@@ -8,9 +8,10 @@ m4/chown.m4
 
 Depends-on:
 open
-unistd
 stat
+stdbool
 sys_stat
+unistd
 
 configure.ac:
 gl_FUNC_CHOWN
index 233e3347907188286d6a2bfcf4294406dac90898..75672b44a17823539e17937752f11e2009bab823 100644 (file)
@@ -9,6 +9,7 @@ Depends-on:
 chown
 errno
 lstat
+stdbool
 sys_stat
 unistd
 
index b0987c51ccd04ef4ad53a2cb1ffe983e245e52d4..a1e8b680995b5e4baae9c5adef40187e0590e056 100644 (file)
@@ -75,6 +75,14 @@ nap (void)
 # define getegid() (-1)
 #endif
 
+#ifndef HAVE_LCHMOD
+# define HAVE_LCHMOD 0
+#endif
+
+#ifndef CHOWN_CHANGE_TIME_BUG
+# define CHOWN_CHANGE_TIME_BUG 0
+#endif
+
 /* This file is designed to test lchown(n,o,g) and
    chownat(AT_FDCWD,n,o,g,AT_SYMLINK_NOFOLLOW).  FUNC is the function
    to test.  Assumes that BASE and ASSERT are already defined, and
@@ -251,8 +259,10 @@ test_lchown (int (*func) (char const *, uid_t, gid_t), bool print)
       ASSERT (st1.st_uid == st2.st_uid);
       ASSERT (gids[0] == st2.st_gid);
     }
-  else
+  else if (!CHOWN_CHANGE_TIME_BUG || HAVE_LCHMOD)
     {
+      /* If we don't have lchmod, and lchown fails to change ctime,
+         then we can't test this part of lchown.  */
       struct stat l1;
       struct stat l2;
       ASSERT (stat (BASE "dir/file", &st1) == 0);