On 6/8/22 13:27, Lance Fredrickson wrote:
Would be nice to see a fix upstream before more projects update gnulib
and the issue becomes broader.
It sounds like a source-code configuration issue, as your platform's
headers in /usr/include correspond to a kernel newer than what you're
running (which is not supported in general).
That being said, it might not hurt to simplify this messy old code so
that it's more portable to messy old platforms. Also, now that I'm
thinking about it, the Gnulib code didn't work if fstatat fails with
EOVERFLOW and should be using readlinkat instead. Please try the first
attached patch, which I've installed in Gnulib. I also installed the 2nd
attached patch which is just a doc update.From 14379aa60449d0b48b9c247391ba863d271f4cb4 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 11 Jun 2022 16:58:25 -0700
Subject: [PATCH 1/2] fchmodat: port to old Linux kernel + newer headers
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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 | 21 ++++++++++++++++++
lib/fchmodat.c | 54 ++++++++++++++++------------------------------
lib/lchmod.c | 56 +++++++++++++++++-------------------------------
modules/fchmodat | 1 -
4 files changed, 59 insertions(+), 73 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 6dbc0b089f..54ac81901d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+2022-06-11 Paul Eggert <egg...@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 <br...@clisp.org>
fopen-gnu: Make this module work again (regression 2022-01-03).
diff --git a/lib/fchmodat.c b/lib/fchmodat.c
index dc53583366..b233c366de 100644
--- a/lib/fchmodat.c
+++ b/lib/fchmodat.c
@@ -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;
diff --git a/lib/lchmod.c b/lib/lchmod.c
index 706dddff7b..b4cc0a8176 100644
--- a/lib/lchmod.c
+++ b/lib/lchmod.c
@@ -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)
diff --git a/modules/fchmodat b/modules/fchmodat
index 26758bda86..a9d17312b6 100644
--- a/modules/fchmodat
+++ b/modules/fchmodat
@@ -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]
--
2.34.1
From 3fa4f598df4dfb29a3609d51713614f76f4acc82 Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Sat, 11 Jun 2022 16:59:12 -0700
Subject: [PATCH 2/2] fcntl: document O_PATH
* doc/posix-headers/fcntl.texi: Mention O_PATH.
---
ChangeLog | 3 +++
doc/posix-headers/fcntl.texi | 6 ++++++
2 files changed, 9 insertions(+)
diff --git a/ChangeLog b/ChangeLog
index 54ac81901d..9975ebc035 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,8 @@
2022-06-11 Paul Eggert <egg...@cs.ucla.edu>
+ fcntl: document O_PATH
+ * doc/posix-headers/fcntl.texi: Mention O_PATH.
+
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
diff --git a/doc/posix-headers/fcntl.texi b/doc/posix-headers/fcntl.texi
index 860dc3f869..8be69f4656 100644
--- a/doc/posix-headers/fcntl.texi
+++ b/doc/posix-headers/fcntl.texi
@@ -86,6 +86,12 @@ Solaris 11.3.
Portability problems not fixed by Gnulib:
@itemize
+
+@item
+@samp{O_PATH} is not defined on some platforms:
+AIX 7.3, FreeBSD 9.3, IRIX 6.5, macOS 13, mingw, Minix 3.3.0, MSVC 14,
+NetBSD 9.2, OpenBSD 7.1, Solaris 11.4.
+
@item
@samp{F_SETFD}, @samp{F_GETFL}, @samp{F_SETFL}, @samp{F_GETLK},
@samp{F_SETLK}, @samp{F_SETLKW}, @samp{F_GETOWN}, and @samp{F_SETOWN}
--
2.34.1
