Re: [PATCH] fchmodat, lchmod: port to buggy Linux filesystems

[Bruno Haible]

Hi Paul,

> +     fchmodat, lchmod: port to buggy Linux filesystems
> +     Problem reported by Florian Weimer in:
> +     https://www.sourceware.org/ml/libc-alpha/2020-02/msg00534.html
> +     * lib/fchmodat.c (fchmodat):
> +     * lib/lchmod.c (lchmod):
> +     Don’t assume that chmod on the O_PATH-opened fd will do
> +     the right thing on a symbolic link.
> +     * lib/fchmodat.c (fchmodat):
> +     Don’t attempt to special-case
> +     any flag value other than AT_SYMLINK_NOFOLLOW.

What about OSes other than Linux?


1) The layout of the /proc file system is specific to Linux. It looks
different on BSD and other systems. Therefore the code block that makes
assumptions about /proc ought to be guarded with 'defined __linux__', no?
Here is a proposed patch:

diff --git a/lib/fchmodat.c b/lib/fchmodat.c
index 02e2da9..180b95d 100644
--- a/lib/fchmodat.c
+++ b/lib/fchmodat.c
@@ -67,7 +67,7 @@ fchmodat (int dir, char const *file, mode_t mode, int flags)
     {
       struct stat st;
 
-# if defined O_PATH && defined AT_EMPTY_PATH
+# if defined O_PATH && defined AT_EMPTY_PATH && defined __linux__
       int fd = openat (dir, file, O_PATH | O_NOFOLLOW | O_CLOEXEC);
       if (fd < 0)
         return fd;
diff --git a/lib/lchmod.c b/lib/lchmod.c
index c7191c0..6802774 100644
--- a/lib/lchmod.c
+++ b/lib/lchmod.c
@@ -37,7 +37,7 @@ lchmod (char const *file, mode_t mode)
 #if HAVE_FCHMODAT
   return fchmodat (AT_FDCWD, file, mode, AT_SYMLINK_NOFOLLOW);
 #else
-# if defined AT_FDCWD && defined O_PATH && defined AT_EMPTY_PATH
+# if defined AT_FDCWD && defined O_PATH && defined AT_EMPTY_PATH && defined 
__linux__
   int fd = openat (AT_FDCWD, file, O_PATH | O_NOFOLLOW | O_CLOEXEC);
   if (fd < 0)
     return fd;


2) Also the discussion what is the "right" behaviour is specific to Linux.
The code in the '#else' case

  if (S_ISLNK (st.st_mode))
    {
      close (fd);
      errno = EOPNOTSUPP;
      return -1;
    }

will surely upset users on BSD systems, where symlinks are intended to have
permission bits.


Bruno