about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFlorian Weimer <fweimer@redhat.com>2020-02-12 13:40:26 +0100
committerFlorian Weimer <fweimer@redhat.com>2020-02-12 20:58:56 +0100
commit1afe8d934bae345de278d79334ef6ab57d3614f7 (patch)
treea629dc36bc1b55d0f7fe815e6eec458a51f6222e
parentf34c4d0f10ed09500d5f0ebd473c3f37ce4989d7 (diff)
downloadglibc-fw/tst-lchmod.tar.gz
glibc-fw/tst-lchmod.tar.xz
glibc-fw/tst-lchmod.zip
Linux: Work around kernel bugs in chmod on /proc/self/fd paths fw/tst-lchmod
It appears that the ability to change symbolic link modes through such
paths is unintended.  On several file systems, the operation fails with
EOPNOTSUPP, even though the symbolic link permissions are updated.
The expected behavior is a failure to update the permissions, without
file system changes.
-rw-r--r--io/tst-lchmod.c83
-rw-r--r--sysdeps/unix/sysv/linux/fchmodat.c28
2 files changed, 49 insertions, 62 deletions
diff --git a/io/tst-lchmod.c b/io/tst-lchmod.c
index 73e45549af..59873f130d 100644
--- a/io/tst-lchmod.c
+++ b/io/tst-lchmod.c
@@ -102,68 +102,44 @@ test_1 (bool do_relative_path, int (*chmod_func) (int fd, const char *, mode_t,
   TEST_VERIFY ((st.st_mode & 0777) != 2);
   mode_t original_symlink_mode = st.st_mode;
 
-  /* Set to true if AT_SYMLINK_NOFOLLOW is supported.  */
-  bool nofollow;
-
   /* We should be able to change the mode of a file, including through
      the symbolic link to-file.  */
   const char *arg = select_path (do_relative_path, path_file, "file");
   TEST_COMPARE (chmod_func (fd, arg, 1, 0), 0);
   xstat (path_file, &st);
   TEST_COMPARE (st.st_mode & 0777, 1);
-  int ret = chmod_func (fd, path_file, 2, AT_SYMLINK_NOFOLLOW);
-  if (ret == 0)
-    {
-      printf ("info: AT_SYMLINK_NOFOLLOW support in %s\n", tempdir);
-      nofollow = true;
-    }
-  else
-    {
-      printf ("info: no AT_SYMLINK_NOFOLLOW support in %s\n", tempdir);
-      nofollow = false;
-
-      /* Set up things for the code below.  */
-      TEST_COMPARE (chmod_func (fd, path_file, 2, 0), 0);
-    }
+  arg = select_path (do_relative_path, path_to_file, "to-file");
+  TEST_COMPARE (chmod_func (fd, path_to_file, 2, 0), 0);
   xstat (path_file, &st);
   TEST_COMPARE (st.st_mode & 0777, 2);
-  arg = select_path (do_relative_path, path_to_file, "to-file");
-  TEST_COMPARE (chmod_func (fd, path_to_file, 1, 0), 0);
+  xlstat (path_to_file, &st);
+  TEST_COMPARE (original_symlink_mode, st.st_mode);
+  arg = select_path (do_relative_path, path_file, "file");
+  TEST_COMPARE (chmod_func (fd, arg, 1, 0), 0);
   xstat (path_file, &st);
   TEST_COMPARE (st.st_mode & 0777, 1);
   xlstat (path_to_file, &st);
   TEST_COMPARE (original_symlink_mode, st.st_mode);
 
-  /* Changing the mode of a symbolic link may fail.  */
+  /* Changing the mode of a symbolic link should fail fail.  */
   arg = select_path (do_relative_path, path_to_file, "to-file");
-  ret = chmod_func (fd, arg, 2, AT_SYMLINK_NOFOLLOW);
-  if (nofollow)
-    {
-      TEST_COMPARE (ret, 0);
-
-      /* The mode of the link changed.  */
-      xlstat (path_to_file, &st);
-      TEST_COMPARE (st.st_mode & 0777, 2);
-
-      /* But the mode of the file is unchanged.  */
-      xstat (path_file, &st);
-      TEST_COMPARE (st.st_mode & 0777, 1);
+  int ret = chmod_func (fd, arg, 2, AT_SYMLINK_NOFOLLOW);
+  TEST_COMPARE (ret, -1);
+  TEST_COMPARE (errno, EOPNOTSUPP);
 
-    }
-  else
-    {
-      TEST_COMPARE (ret, -1);
-      TEST_COMPARE (errno, EOPNOTSUPP);
+  /* The modes should remain unchanged.  */
+  xstat (path_file, &st);
+  TEST_COMPARE (st.st_mode & 0777, 1);
+  xlstat (path_to_file, &st);
+  TEST_COMPARE (original_symlink_mode, st.st_mode);
 
-      /* The modes should remain unchanged.  */
-      xstat (path_file, &st);
-      TEST_COMPARE (st.st_mode & 0777, 1);
-      xlstat (path_to_file, &st);
-      TEST_COMPARE (original_symlink_mode, st.st_mode);
-    }
+  arg = select_path (do_relative_path, path_to_file, "to-file");
+  ret = chmod_func (fd, arg, 2, AT_SYMLINK_NOFOLLOW);
+  TEST_COMPARE (ret, -1);
+  TEST_COMPARE (errno, EOPNOTSUPP);
 
-  /* If we have NOFOLLOW support, we should be able to change the mode
-     of a dangling symbolic link or a symbolic link loop.  */
+  /* Likewise, changing dangling and looping symbolic links must
+     fail.  */
   const char *paths[] = { path_dangling, path_loop };
   for (size_t i = 0; i < array_length (paths); ++i)
     {
@@ -178,19 +154,10 @@ test_1 (bool do_relative_path, int (*chmod_func) (int fd, const char *, mode_t,
       original_symlink_mode = st.st_mode;
       arg = select_path (do_relative_path, path, filename);
       ret = chmod_func (fd, arg, new_mode, AT_SYMLINK_NOFOLLOW);
-      if (nofollow)
-        {
-          TEST_COMPARE (ret, 0);
-          xlstat (path, &st);
-          TEST_COMPARE (st.st_mode & 0777, new_mode);
-        }
-      else /* !nofollow.  */
-        {
-          TEST_COMPARE (ret, -1);
-          TEST_COMPARE (errno, EOPNOTSUPP);
-          xlstat (path, &st);
-          TEST_COMPARE (st.st_mode, original_symlink_mode);
-        }
+      TEST_COMPARE (ret, -1);
+      TEST_COMPARE (errno, EOPNOTSUPP);
+      xlstat (path, &st);
+      TEST_COMPARE (st.st_mode, original_symlink_mode);
     }
 
    /* A missing file should always result in ENOENT.  The presence of
diff --git a/sysdeps/unix/sysv/linux/fchmodat.c b/sysdeps/unix/sysv/linux/fchmodat.c
index 719053b333..17eca54051 100644
--- a/sysdeps/unix/sysv/linux/fchmodat.c
+++ b/sysdeps/unix/sysv/linux/fchmodat.c
@@ -45,6 +45,30 @@ fchmodat (int fd, const char *file, mode_t mode, int flag)
 	   caller can treat them as temporary if necessary.  */
 	return pathfd;
 
+      /* Use fstatat because fstat does not work on O_PATH descriptors
+	 before Linux 3.6.  */
+      struct stat64 st;
+      if (fstatat64 (pathfd, "", &st, AT_EMPTY_PATH) != 0)
+	{
+	  __close_nocancel (pathfd);
+	  return -1;
+	}
+
+      /* Some Linux versions with some file systems can actually
+	 change symbolic link permissions via /proc, but this is not
+	 intentional, and it gives inconsistent results (e.g., error
+	 return despite mode change).  The expected behavior is that
+	 symbolic link modes cannot be changed at all, and this check
+	 enforces that.  */
+      if (S_ISLNK (st.st_mode))
+	{
+	  __close_nocancel (pathfd);
+	  __set_errno (EOPNOTSUPP);
+	  return -1;
+	}
+
+      /* For most file systems, fchmod does not operate on O_PATH
+	 descriptors, so go through /proc.  */
       char buf[32];
       if (__snprintf (buf, sizeof (buf), "/proc/self/fd/%d", pathfd) < 0)
 	{
@@ -54,10 +78,6 @@ fchmodat (int fd, const char *file, mode_t mode, int flag)
 	  return -1;
 	}
 
-      /* This operates directly on the symbolic link if it is one.
-	 /proc/self/fd files look like symbolic links, but they are
-	 not.  (fchmod and fchmodat do not work on O_PATH descriptors,
-	 similar to fstat before Linux 3.6.)  */
       int ret = __chmod (buf, mode);
       if (ret != 0)
 	{