From 20527b26f5e020ebaa48a8d5daec4cccebf051df Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Mon, 28 Dec 2020 12:38:52 -0800 Subject: [PATCH 3/3] faccessat: revert recent EOVERFLOW change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I misunderstood the glibc source code. Deduced from Adhemerval Zanella’s proposed glibc patch in: https://sourceware.org/pipermail/libc-alpha/2020-December/121131.html * doc/posix-functions/faccessat.texi: It is not a problem. * lib/canonicalize-lgpl.c, lib/canonicalize.c, lib/faccessat.c: (FACCESSAT_NEVER_OVERFLOWS): Remove. All uses removed. * lib/faccessat.c: Revert to simpler version now that LSTAT_FOLLOWS_SLASHED_SYMLINK must be false. * m4/faccessat.m4 (gl_FUNC_FACCESSAT_EOVERFLOW): Remove. All uses removed. * modules/canonicalize, modules/canonicalize-lgpl (Files): Remove m4/faccessat.m4. --- ChangeLog | 14 +++++++++++++ doc/posix-functions/faccessat.texi | 14 +++---------- lib/canonicalize-lgpl.c | 16 ++------------- lib/canonicalize.c | 10 ++------- lib/faccessat.c | 14 +------------ m4/canonicalize.m4 | 4 +--- m4/faccessat.m4 | 33 +++--------------------------- modules/canonicalize | 1 - modules/canonicalize-lgpl | 1 - 9 files changed, 26 insertions(+), 81 deletions(-) diff --git a/ChangeLog b/ChangeLog index 9a91eda92..1481becc9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,19 @@ 2020-12-28 Paul Eggert + faccessat: revert recent EOVERFLOW change + I misunderstood the glibc source code. Deduced from + Adhemerval Zanella’s proposed glibc patch in: + https://sourceware.org/pipermail/libc-alpha/2020-December/121131.html + * doc/posix-functions/faccessat.texi: It is not a problem. + * lib/canonicalize-lgpl.c, lib/canonicalize.c, lib/faccessat.c: + (FACCESSAT_NEVER_OVERFLOWS): Remove. All uses removed. + * lib/faccessat.c: Revert to simpler version now that + LSTAT_FOLLOWS_SLASHED_SYMLINK must be false. + * m4/faccessat.m4 (gl_FUNC_FACCESSAT_EOVERFLOW): + Remove. All uses removed. + * modules/canonicalize, modules/canonicalize-lgpl (Files): + Remove m4/faccessat.m4. + canonicalize-lgpl: accommodate picky cpp * lib/canonicalize-lgpl.c: Use "defined FUNC_REALPATH_WORKS" in case preprocessor is picky. Reported by Adhemerval Zanella in: diff --git a/doc/posix-functions/faccessat.texi b/doc/posix-functions/faccessat.texi index 07ea8e7bf..5d5165e47 100644 --- a/doc/posix-functions/faccessat.texi +++ b/doc/posix-functions/faccessat.texi @@ -15,12 +15,6 @@ glibc 2.3.6, macOS 10.12, FreeBSD 7.4, NetBSD 6.1.5, OpenBSD 4.9, Minix 3.1.8, A On some platforms, @code{faccessat (dfd, "file/", amode, flag)} succeeds instead of failing when @file{file} is not a directory. macOS 10.13. -@item -On some platforms, @code{faccessat} can incorrectly fail with -@code{EOVERFLOW} when the mode is @code{F_OK}: -GNU/Linux with glibc 2.32, or with Linux kernel 5.7. -@c This bug should be fixed in glibc 2.33 and kernel 5.8. See: -@c https://sourceware.org/bugzilla/show_bug.cgi?id=18683 @end itemize Portability problems not fixed by Gnulib: @@ -36,11 +30,9 @@ The replacement does not support the @code{AT_SYMLINK_NOFOLLOW} flag, which is supported by GNU @code{faccessat}. @item On some platforms, @code{faccessat} can mishandle @code{AT_EACCESS} -after a process starts as root and then becomes non-root, -or can incorrectly fail with @code{EOVERFLOW} when the mode -is not @code{F_OK}: -GNU/Linux with glibc 2.32, or with Linux kernel 5.7. -@c These bugs should be fixed in glibc 2.33 and kernel 5.8. See: +after a process starts as root and then becomes non-root: +GNU/Linux with glibc 2.32. +@c This bug should be fixed in glibc 2.33. See: @c https://sourceware.org/bugzilla/show_bug.cgi?id=18683 @end itemize diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 332b5bab4..04fe95253 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -44,12 +44,6 @@ #ifdef _LIBC # include -# include -# ifdef __ASSUME_FACCESSAT2 -# define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2 -# else -# define FACCESSAT_NEVER_EOVERFLOWS true -# endif # define GCC_LINT 1 # define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__)) #else @@ -94,9 +88,6 @@ #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false #endif -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS false -#endif #if defined _LIBC || !FUNC_REALPATH_WORKS @@ -106,14 +97,11 @@ static bool file_accessible (char const *file) { # if defined _LIBC || HAVE_FACCESSAT - int r = __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); + return __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS) == 0; # else struct stat st; - int r = __stat (file, &st); + return __stat (file, &st) == 0 || errno == EOVERFLOW; # endif - - return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW) - || r == 0); } /* True if concatenating END as a suffix to a file name means that the diff --git a/lib/canonicalize.c b/lib/canonicalize.c index 48a49e412..a4d3aab96 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -46,9 +46,6 @@ #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT # define DOUBLE_SLASH_IS_DISTINCT_ROOT false #endif -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS false -#endif #if ISSLASH ('\\') # define SLASHES "/\\" @@ -62,14 +59,11 @@ static bool file_accessible (char const *file) { # if HAVE_FACCESSAT - int r = faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); + return faccessat (AT_FDCWD, file, F_OK, AT_EACCESS) == 0; # else struct stat st; - int r = stat (file, &st); + return stat (file, &st) == 0 || errno == EOVERFLOW; # endif - - return ((!FACCESSAT_NEVER_EOVERFLOWS && r < 0 && errno == EOVERFLOW) - || r == 0); } /* True if concatenating END as a suffix to a file name means that the diff --git a/lib/faccessat.c b/lib/faccessat.c index 330c54a0b..9f6a11bf6 100644 --- a/lib/faccessat.c +++ b/lib/faccessat.c @@ -32,13 +32,6 @@ #include #undef _GL_INCLUDING_UNISTD_H -#ifndef FACCESSAT_NEVER_EOVERFLOWS -# define FACCESSAT_NEVER_EOVERFLOWS 0 -#endif -#ifndef LSTAT_FOLLOWS_SLASHED_SYMLINK -# define LSTAT_FOLLOWS_SLASHED_SYMLINK 0 -#endif - #if HAVE_FACCESSAT static int orig_faccessat (int fd, char const *name, int mode, int flag) @@ -66,12 +59,7 @@ rpl_faccessat (int fd, char const *file, int mode, int flag) { int result = orig_faccessat (fd, file, mode, flag); - if (result != 0) - { - if (!FACCESSAT_NEVER_EOVERFLOWS && mode == F_OK && errno == EOVERFLOW) - return 0; - } - else if (!LSTAT_FOLLOWS_SLASHED_SYMLINK && file[strlen (file) - 1] == '/') + if (result == 0 && file[strlen (file) - 1] == '/') { struct stat st; result = fstatat (fd, file, &st, 0); diff --git a/m4/canonicalize.m4 b/m4/canonicalize.m4 index c8da4dfcb..404db4cb6 100644 --- a/m4/canonicalize.m4 +++ b/m4/canonicalize.m4 @@ -1,4 +1,4 @@ -# canonicalize.m4 serial 34 +# canonicalize.m4 serial 35 dnl Copyright (C) 2003-2007, 2009-2020 Free Software Foundation, Inc. @@ -11,7 +11,6 @@ dnl with or without modifications, as long as this notice is preserved. AC_DEFUN([gl_FUNC_CANONICALIZE_FILENAME_MODE], [ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat]) AC_REQUIRE([gl_DOUBLE_SLASH_ROOT]) @@ -58,7 +57,6 @@ AC_DEFUN([gl_CANONICALIZE_LGPL], AC_DEFUN([gl_CANONICALIZE_LGPL_SEPARATE], [ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat]) diff --git a/m4/faccessat.m4 b/m4/faccessat.m4 index a4ad31a46..b8da51e4c 100644 --- a/m4/faccessat.m4 +++ b/m4/faccessat.m4 @@ -1,4 +1,4 @@ -# serial 9 +# serial 10 # See if we need to provide faccessat replacement. dnl Copyright (C) 2009-2020 Free Software Foundation, Inc. @@ -8,31 +8,6 @@ dnl with or without modifications, as long as this notice is preserved. # Written by Eric Blake. -AC_DEFUN([gl_FUNC_FACCESSAT_EOVERFLOW], -[ - AC_CHECK_FUNCS_ONCE([faccessat]) - if test "$ac_cv_func_faccessat" = yes; then - AC_CACHE_CHECK([whether faccessat works when stat would EOVERFLOW], - [gl_cv_func_faccessat_never_eoverflows], - [AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM([], - [[#ifdef __linux__ - #include - #if (! (KERNEL_VERSION (5, 8, 0) <= LINUX_VERSION_CODE \ - && 2 < (__GLIBC__ + (33 <= __GLIBC_MINOR__)))) - #error "faccessat might fail with EOVERFLOW" - #endif - #endif - ]])], - [gl_cv_func_faccessat_never_eoverflows=yes], - [gl_cv_func_faccessat_never_eoverflows=no])]) - if test "$gl_cv_func_faccessat_never_eoverflows" = yes; then - AC_DEFINE([FACCESSAT_NEVER_EOVERFLOWS], 1, - [Define to 1 if faccessat is EOVERFLOW-free.]) - fi - fi -]) - AC_DEFUN([gl_FUNC_FACCESSAT], [ AC_REQUIRE([gl_UNISTD_H_DEFAULTS]) @@ -41,14 +16,12 @@ AC_DEFUN([gl_FUNC_FACCESSAT], dnl Persuade glibc to declare faccessat(). AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) - AC_CHECK_FUNCS_ONCE([faccessat]) if test $ac_cv_func_faccessat = no; then HAVE_FACCESSAT=0 else - case $gl_cv_func_lstat_dereferences_slashed_symlink,$gl_cv_func_faccessat_never_eoverflows in - *yes,*yes) ;; + case $gl_cv_func_lstat_dereferences_slashed_symlink in + *yes) ;; *) REPLACE_FACCESSAT=1 ;; esac fi diff --git a/modules/canonicalize b/modules/canonicalize index 29919bcdc..5003f2682 100644 --- a/modules/canonicalize +++ b/modules/canonicalize @@ -5,7 +5,6 @@ Files: lib/canonicalize.h lib/canonicalize.c m4/canonicalize.m4 -m4/faccessat.m4 m4/lstat.m4 Depends-on: diff --git a/modules/canonicalize-lgpl b/modules/canonicalize-lgpl index b5f3e7f69..a96f9011e 100644 --- a/modules/canonicalize-lgpl +++ b/modules/canonicalize-lgpl @@ -5,7 +5,6 @@ Files: lib/canonicalize-lgpl.c m4/canonicalize.m4 m4/double-slash-root.m4 -m4/faccessat.m4 m4/lstat.m4 Depends-on: -- 2.27.0