]> 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)
committerPaul Eggert <eggert@cs.ucla.edu>
Sun, 12 Jun 2022 00:00:37 +0000 (17:00 -0700)
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 6dbc0b089f4b232a234295dd3d6dd146e97d76fa..54ac81901d4fdb9b1f782704d9866ee7ca1519b3 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 dc535833660abb15989e0d7ba8cdbe23232cc20a..b233c366de70b418b5afa64a6812a7647c9afa37 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 706dddff7bb806e08db16cbaed81727271ae39a3..b4cc0a8176c88509a7b3f81bd8762b0bd306e7ab 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 26758bda861c6f7d5870c86b7c977fd98e9b2e85..a9d17312b6a1bfa797bd3b1f0d554dddf8216e2b 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]