]> Savannah Git Hosting - gnulib.git/commitdiff
fchmodat: port to old Linux kernel + newer headers
authorPaul Eggert <eggert@cs.ucla.edu>
Sat, 11 Jun 2022 23:58:25 +0000 (16:58 -0700)
committerBruno Haible <bruno@clisp.org>
Wed, 31 Aug 2022 23:09:35 +0000 (01:09 +0200)
Problem reported by Lance Fredrickson in:
https://lists.gnu.org/r/bug-gnulib/2022-06/msg00038.html
* lib/fchmodat.c (fchmodat):
* lib/lchmod.c (lchmod): Do not rely on AT_EMPTY_PATH as to
whether syscalls work on ""; instead, if a call fails with
ENOENT assume that those syscalls do not work.
Do not use fstatat to determine whether a file is a symlink,
as this has problems with EOVERFLOW.  Use readlinkat instead,
and if it fails with EINVAL then the file is not a symlink.
Remove #if tests on __linux__ || __ANDROID__ || __CYGWIN__
as this has been a maintenance hassle and it’s unlikely
these days that a new platform would #define O_PATH without also
either supporting /proc or keeping it absent.
* modules/fchmodat (Depends-on): Remove fstatat.
There should be no need for either fchmodat or lchmod to depend on
readlinkat, since they use readlinkat only in contexts where it
should work without Gnulib intervention.

ChangeLog
lib/fchmodat.c
lib/lchmod.c
modules/fchmodat

index ae99d319c92902a520c33f99072a4d399966331d..13cd60378f73a3c8a5be51b20be7878dd0bb68df 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2022-06-11  Paul Eggert  <eggert@cs.ucla.edu>
+
+       fchmodat: port to old Linux kernel + newer headers
+       Problem reported by Lance Fredrickson in:
+       https://lists.gnu.org/r/bug-gnulib/2022-06/msg00038.html
+       * lib/fchmodat.c (fchmodat):
+       * lib/lchmod.c (lchmod): Do not rely on AT_EMPTY_PATH as to
+       whether syscalls work on ""; instead, if a call fails with
+       ENOENT assume that those syscalls do not work.
+       Do not use fstatat to determine whether a file is a symlink,
+       as this has problems with EOVERFLOW.  Use readlinkat instead,
+       and if it fails with EINVAL then the file is not a symlink.
+       Remove #if tests on __linux__ || __ANDROID__ || __CYGWIN__
+       as this has been a maintenance hassle and it’s unlikely
+       these days that a new platform would #define O_PATH without also
+       either supporting /proc or keeping it absent.
+       * modules/fchmodat (Depends-on): Remove fstatat.
+       There should be no need for either fchmodat or lchmod to depend on
+       readlinkat, since they use readlinkat only in contexts where it
+       should work without Gnulib intervention.
+
 2022-06-06  Bruno Haible  <bruno@clisp.org>
 
        fopen-gnu: Make this module work again (regression 2022-01-03).
index 506e6badd7dde49397614924c25dc21ed3bb53f3..3531b9030fd1bf7d2dbab1dab5127a5efd1a4b6f 100644 (file)
@@ -85,7 +85,7 @@ fchmodat (int dir, char const *file, mode_t mode, int flags)
     {
       struct stat st;
 
-#  if defined O_PATH && defined AT_EMPTY_PATH
+#  ifdef O_PATH
       /* Open a file descriptor with O_NOFOLLOW, to make sure we don't
          follow symbolic links, if /proc is mounted.  O_PATH is used to
          avoid a failure if the file is not readable.
@@ -94,45 +94,28 @@ fchmodat (int dir, char const *file, mode_t mode, int flags)
       if (fd < 0)
         return fd;
 
-      /* Up to Linux 5.3 at least, when FILE refers to a symbolic link, the
-         chmod call below will change the permissions of the symbolic link
-         - which is undesired - and on many file systems (ext4, btrfs, jfs,
-         xfs, ..., but not reiserfs) fail with error EOPNOTSUPP - which is
-         misleading.  Therefore test for a symbolic link explicitly.
-         Use fstatat because fstat does not work on O_PATH descriptors
-         before Linux 3.6.  */
-      if (fstatat (fd, "", &st, AT_EMPTY_PATH) != 0)
+      int err;
+      char buf[1];
+      if (0 <= readlinkat (fd, "", buf, sizeof buf))
+        err = EOPNOTSUPP;
+      else if (errno == EINVAL)
         {
-          int stat_errno = errno;
-          close (fd);
-          errno = stat_errno;
-          return -1;
-        }
-      if (S_ISLNK (st.st_mode))
-        {
-          close (fd);
-          errno = EOPNOTSUPP;
-          return -1;
+          static char const fmt[] = "/proc/self/fd/%d";
+          char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
+          sprintf (buf, fmt, fd);
+          err = chmod (buf, mode) == 0 ? 0 : errno == ENOENT ? -1 : errno;
         }
+      else
+        err = errno == ENOENT ? -1 : errno;
 
-#   if defined __linux__ || defined __ANDROID__ || defined __CYGWIN__
-      static char const fmt[] = "/proc/self/fd/%d";
-      char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
-      sprintf (buf, fmt, fd);
-      int chmod_result = chmod (buf, mode);
-      int chmod_errno = errno;
       close (fd);
-      if (chmod_result == 0)
-        return chmod_result;
-      if (chmod_errno != ENOENT)
-        {
-          errno = chmod_errno;
-          return chmod_result;
-        }
-#   endif
-      /* /proc is not mounted or would not work as in GNU/Linux.  */
 
-#  else
+      errno = err;
+      if (0 <= err)
+        return err == 0 ? 0 : -1;
+#  endif
+
+      /* O_PATH + /proc is not supported.  */
       int fstatat_result = fstatat (dir, file, &st, AT_SYMLINK_NOFOLLOW);
       if (fstatat_result != 0)
         return fstatat_result;
@@ -141,7 +124,6 @@ fchmodat (int dir, char const *file, mode_t mode, int flags)
           errno = EOPNOTSUPP;
           return -1;
         }
-#  endif
 
       /* Fall back on orig_fchmodat with no flags, despite a possible race.  */
       flags = 0;
index 479ed776cbaa3ea558791bc42de615e5a83ac3bc..b6ff81c7134967ab11dda3ee232d66b19df24d2a 100644 (file)
@@ -45,7 +45,7 @@
 int
 lchmod (char const *file, mode_t mode)
 {
-#if defined O_PATH && defined AT_EMPTY_PATH
+#ifdef O_PATH
   /* Open a file descriptor with O_NOFOLLOW, to make sure we don't
      follow symbolic links, if /proc is mounted.  O_PATH is used to
      avoid a failure if the file is not readable.
@@ -54,46 +54,30 @@ lchmod (char const *file, mode_t mode)
   if (fd < 0)
     return fd;
 
-  /* Up to Linux 5.3 at least, when FILE refers to a symbolic link, the
-     chmod call below will change the permissions of the symbolic link
-     - which is undesired - and on many file systems (ext4, btrfs, jfs,
-     xfs, ..., but not reiserfs) fail with error EOPNOTSUPP - which is
-     misleading.  Therefore test for a symbolic link explicitly.
-     Use fstatat because fstat does not work on O_PATH descriptors
-     before Linux 3.6.  */
-  struct stat st;
-  if (fstatat (fd, "", &st, AT_EMPTY_PATH) != 0)
-    {
-      int stat_errno = errno;
-      close (fd);
-      errno = stat_errno;
-      return -1;
-    }
-  if (S_ISLNK (st.st_mode))
+  int err;
+  char buf[1];
+  if (0 <= readlinkat (fd, "", buf, sizeof buf))
+    err = EOPNOTSUPP;
+  else if (errno == EINVAL)
     {
-      close (fd);
-      errno = EOPNOTSUPP;
-      return -1;
+      static char const fmt[] = "/proc/self/fd/%d";
+      char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
+      sprintf (buf, fmt, fd);
+      err = chmod (buf, mode) == 0 ? 0 : errno == ENOENT ? -1 : errno;
     }
+  else
+    err = errno == ENOENT ? -1 : errno;
 
-# if defined __linux__ || defined __ANDROID__ || defined __CYGWIN__
-  static char const fmt[] = "/proc/self/fd/%d";
-  char buf[sizeof fmt - sizeof "%d" + INT_BUFSIZE_BOUND (int)];
-  sprintf (buf, fmt, fd);
-  int chmod_result = chmod (buf, mode);
-  int chmod_errno = errno;
   close (fd);
-  if (chmod_result == 0)
-    return chmod_result;
-  if (chmod_errno != ENOENT)
-    {
-      errno = chmod_errno;
-      return chmod_result;
-    }
-# endif
-  /* /proc is not mounted or would not work as in GNU/Linux.  */
 
-#elif HAVE_LSTAT
+  errno = err;
+  if (0 <= err)
+    return err == 0 ? 0 : -1;
+#endif
+
+  /* O_PATH + /proc is not supported.  */
+
+#if HAVE_LSTAT
   struct stat st;
   int lstat_result = lstat (file, &st);
   if (lstat_result != 0)
index 17cfe392fdd2ecebd5402595efa5ef7acea10388..072d148be1c86806ee7aab419c5abf8528a6c09d 100644 (file)
@@ -14,7 +14,6 @@ fcntl-h         [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 unistd          [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 intprops        [test $HAVE_FCHMODAT = 0 || test $REPLACE_FCHMODAT = 1]
 c99             [test $REPLACE_FCHMODAT = 1]
-fstatat         [test $REPLACE_FCHMODAT = 1]
 at-internal     [test $HAVE_FCHMODAT = 0]
 extern-inline   [test $HAVE_FCHMODAT = 0]
 fchdir          [test $HAVE_FCHMODAT = 0]