From 14379aa60449d0b48b9c247391ba863d271f4cb4 Mon Sep 17 00:00:00 2001 From: Paul Eggert 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 + + 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 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