>From 225c66111c76144e7320dfdaa831a442dece190b Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Thu, 24 Dec 2020 11:38:48 -0800 Subject: [PATCH 05/10] canonicalize: prefer faccessat to stat A proper faccessat doesn't have the EOVERFLOW problem, and can be more efficient as it needn't gather data from the filesystem to fill in struct stat. So use stat only if faccessat is absent, or when checking for symlink loops in canonicalize.c. * lib/canonicalize-lgpl.c, lib/canonicalize.c: Include fcntl.h, for AT_EACCESS. (FACCESSAT_NEVER_EOVERFLOWS): Default to false. (file_accessible): New function, based on faccessat but with a fallback to stat and with an EOVERFLOW workaround. (dir_check): Use it. (dir_suffix): New static constant. * lib/canonicalize-lgpl.c (FACCESSAT_NEVER_EOVERFLOWS) [_LIBC]: Use __ASSUME_FACCESSAT2 to set FACCESSAT_NEVER_EOVERFLOWS (__faccessat) [!_LIBC]: Define. (realpath_stk): Use dir_suffix now. * lib/canonicalize.c (canonicalize_filename_mode_stk): If logical, don't check each component's existence; just check at the end, as that's enough. * m4/canonicalize.m4 (gl_FUNC_CANONICALIZE_FILENAME_MODE) (gl_CANONICALIZE_LGPL_SEPARATE): Require gl_FUNC_FACCESSAT_EOVERFLOW, gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK, and check for faccessat. (gl_CANONICALIZE_LGPL_SEPARATE): Do not check for readlink, as the code does not use HAVE_READLINK. * modules/canonicalize, modules/canonicalize-lgpl (Files): Add m4/faccessat.m4, m4/lstat.m4. (Depends-on): Add fcntl-lh. --- ChangeLog | 29 +++++++++++++++++++++ lib/canonicalize-lgpl.c | 55 ++++++++++++++++++++++++++++++++------- lib/canonicalize.c | 53 ++++++++++++++++++++++++++++--------- m4/canonicalize.m4 | 10 ++++--- modules/canonicalize | 3 +++ modules/canonicalize-lgpl | 3 +++ 6 files changed, 128 insertions(+), 25 deletions(-) diff --git a/ChangeLog b/ChangeLog index 551059e35..94427d748 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,34 @@ 2020-12-24 Paul Eggert + canonicalize: prefer faccessat to stat + A proper faccessat doesn't have the EOVERFLOW problem, and can be + more efficient as it needn't gather data from the filesystem to + fill in struct stat. So use stat only if faccessat is absent, + or when checking for symlink loops in canonicalize.c. + * lib/canonicalize-lgpl.c, lib/canonicalize.c: + Include fcntl.h, for AT_EACCESS. + (FACCESSAT_NEVER_EOVERFLOWS): Default to false. + (file_accessible): New function, based on faccessat but with + a fallback to stat and with an EOVERFLOW workaround. + (dir_check): Use it. + (dir_suffix): New static constant. + * lib/canonicalize-lgpl.c (FACCESSAT_NEVER_EOVERFLOWS) [_LIBC]: + Use __ASSUME_FACCESSAT2 to set FACCESSAT_NEVER_EOVERFLOWS + (__faccessat) [!_LIBC]: Define. + (realpath_stk): Use dir_suffix now. + * lib/canonicalize.c (canonicalize_filename_mode_stk): + If logical, don't check each component's existence; just check + at the end, as that's enough. + * m4/canonicalize.m4 (gl_FUNC_CANONICALIZE_FILENAME_MODE) + (gl_CANONICALIZE_LGPL_SEPARATE): + Require gl_FUNC_FACCESSAT_EOVERFLOW, + gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK, and check for faccessat. + (gl_CANONICALIZE_LGPL_SEPARATE): Do not check for readlink, + as the code does not use HAVE_READLINK. + * modules/canonicalize, modules/canonicalize-lgpl (Files): + Add m4/faccessat.m4, m4/lstat.m4. + (Depends-on): Add fcntl-lh. + faccessat: work around F_OK EOVERFLOW bug * doc/posix-functions/faccessat.texi: Mention the problem. * lib/faccessat.c (FACCESSAT_NEVER_EOVERFLOWS): Default to 0. diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c index 431dc5a3b..3d4218516 100644 --- a/lib/canonicalize-lgpl.c +++ b/lib/canonicalize-lgpl.c @@ -29,6 +29,7 @@ #include #include +#include #include #include #include @@ -46,12 +47,19 @@ typedef ptrdiff_t idx_t; # define FILE_SYSTEM_PREFIX_LEN(name) 0 # define IS_ABSOLUTE_FILE_NAME(name) ISSLASH(*(name)) # define ISSLASH(c) ((c) == '/') +# include +# ifdef __ASSUME_FACCESSAT2 +# define FACCESSAT_NEVER_EOVERFLOWS __ASSUME_FACCESSAT2 +# else +# define FACCESSAT_NEVER_EOVERFLOWS true +# endif #else # define __canonicalize_file_name canonicalize_file_name # define __realpath realpath # include "idx.h" # include "pathmax.h" # include "filename.h" +# define __faccessat faccessat # if defined _WIN32 && !defined __CYGWIN__ # define __getcwd _getcwd # elif HAVE_GETCWD @@ -88,11 +96,30 @@ typedef ptrdiff_t idx_t; #endif #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT -# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0 +# define DOUBLE_SLASH_IS_DISTINCT_ROOT false +#endif +#ifndef FACCESSAT_NEVER_EOVERFLOWS +# define FACCESSAT_NEVER_EOVERFLOWS false #endif #if !FUNC_REALPATH_WORKS || defined _LIBC +/* Return true if FILE's existence can be shown, false (setting errno) + otherwise. Follow symbolic links. */ +static bool +file_accessible (char const *file) +{ +# if defined _LIBC || HAVE_FACCESSAT + int r = __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); +# else + struct stat st; + int r = __stat (file, &st); +# 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 code needs to check that the file name is that of a searchable directory, since the canonicalize_filename_mode_stk code won't @@ -125,18 +152,26 @@ suffix_requires_dir_check (char const *end) return false; } -/* Return true if DIR is a directory, false (setting errno) otherwise. +/* Append this to a file name to test whether it is a searchable directory. + On POSIX platforms "/" suffices, but "/./" is sometimes needed on + macOS 10.13 , and should also work on + platforms like AIX 7.2 that need at least "/.". */ + +#if defined _LIBC || defined LSTAT_FOLLOWS_SLASHED_SYMLINK +static char const dir_suffix[] = "/"; +#else +static char const dir_suffix[] = "/./"; +#endif + +/* Return true if DIR is a searchable dir, false (setting errno) otherwise. DIREND points to the NUL byte at the end of the DIR string. - Store garbage into DIREND[0] and DIREND[1]. */ + Store garbage into DIREND[0 .. strlen (dir_suffix)]. */ static bool dir_check (char *dir, char *dirend) { - /* Append "/"; otherwise EOVERFLOW would be ambiguous. */ - strcpy (dirend, "/"); - - struct stat st; - return __stat (dir, &st) == 0 || errno == EOVERFLOW; + strcpy (dirend, dir_suffix); + return file_accessible (dir); } static idx_t @@ -280,8 +315,8 @@ realpath_stk (const char *name, char *resolved, if (!ISSLASH (dest[-1])) *dest++ = '/'; - enum { dir_check_room = sizeof "/" }; - while (rname + rname_buf->length - dest < startlen + dir_check_room) + while (rname + rname_buf->length - dest + < startlen + sizeof dir_suffix) { idx_t dest_offset = dest - rname; if (!scratch_buffer_grow_preserve (rname_buf)) diff --git a/lib/canonicalize.c b/lib/canonicalize.c index 62777219a..2c88335bf 100644 --- a/lib/canonicalize.c +++ b/lib/canonicalize.c @@ -19,6 +19,7 @@ #include "canonicalize.h" #include +#include #include #include #include @@ -36,7 +37,10 @@ #include "filename.h" #ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT -# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0 +# define DOUBLE_SLASH_IS_DISTINCT_ROOT false +#endif +#ifndef FACCESSAT_NEVER_EOVERFLOWS +# define FACCESSAT_NEVER_EOVERFLOWS false #endif #if ISSLASH ('\\') @@ -45,6 +49,22 @@ # define SLASHES "/" #endif +/* Return true if FILE's existence can be shown, false (setting errno) + otherwise. Follow symbolic links. */ +static bool +file_accessible (char const *file) +{ +# if HAVE_FACCESSAT + int r = faccessat (AT_FDCWD, file, F_OK, AT_EACCESS); +# else + struct stat st; + int r = stat (file, &st); +# 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 code needs to check that the file name is that of a searchable directory, since the canonicalize_filename_mode_stk code won't @@ -77,18 +97,26 @@ suffix_requires_dir_check (char const *end) return false; } -/* Return true if DIR is a directory, false (setting errno) otherwise. +/* Append this to a file name to test whether it is a searchable directory. + On POSIX platforms "/" suffices, but "/./" is sometimes needed on + macOS 10.13 , and should also work on + platforms like AIX 7.2 that need at least "/.". */ + +#ifdef LSTAT_FOLLOWS_SLASHED_SYMLINK +static char const dir_suffix[] = "/"; +#else +static char const dir_suffix[] = "/./"; +#endif + +/* Return true if DIR is a searchable dir, false (setting errno) otherwise. DIREND points to the NUL byte at the end of the DIR string. - Store garbage into DIREND[0] and DIREND[1]. */ + Store garbage into DIREND[0 .. strlen (dir_suffix)]. */ static bool dir_check (char *dir, char *dirend) { - /* Append "/"; otherwise EOVERFLOW would be ambiguous. */ - strcpy (dirend, "/"); - - struct stat st; - return stat (dir, &st) == 0 || errno == EOVERFLOW; + strcpy (dirend, dir_suffix); + return file_accessible (dir); } #if !((HAVE_CANONICALIZE_FILE_NAME && FUNC_REALPATH_WORKS) \ @@ -293,8 +321,8 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode, if (!ISSLASH (dest[-1])) *dest++ = '/'; - enum { dir_check_room = sizeof "/" }; - while (rname + rname_buf->length - dest < startlen + dir_check_room) + while (rname + rname_buf->length - dest + < startlen + sizeof dir_suffix) { idx_t dest_offset = dest - rname; if (!scratch_buffer_grow_preserve (rname_buf)) @@ -406,8 +434,9 @@ canonicalize_filename_mode_stk (const char *name, canonicalize_mode_t can_mode, else if (! (can_exist == CAN_MISSING || (suffix_requires_dir_check (end) ? dir_check (rname, dest) - : ((logical && 0 <= readlink (rname, &discard, 1)) - || errno == EINVAL)) + : !logical + ? errno == EINVAL + : *end || file_accessible (rname)) || (can_exist == CAN_ALL_BUT_LAST && errno == ENOENT && !end[strspn (end, SLASHES)]))) diff --git a/m4/canonicalize.m4 b/m4/canonicalize.m4 index 14ea3e12f..c8da4dfcb 100644 --- a/m4/canonicalize.m4 +++ b/m4/canonicalize.m4 @@ -1,4 +1,4 @@ -# canonicalize.m4 serial 33 +# canonicalize.m4 serial 34 dnl Copyright (C) 2003-2007, 2009-2020 Free Software Foundation, Inc. @@ -11,7 +11,9 @@ 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_CHECK_FUNCS_ONCE([canonicalize_file_name]) + 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]) AC_REQUIRE([gl_FUNC_REALPATH_WORKS]) if test $ac_cv_func_canonicalize_file_name = no; then @@ -56,7 +58,9 @@ AC_DEFUN([gl_CANONICALIZE_LGPL], AC_DEFUN([gl_CANONICALIZE_LGPL_SEPARATE], [ AC_REQUIRE([gl_USE_SYSTEM_EXTENSIONS]) - AC_CHECK_FUNCS_ONCE([canonicalize_file_name readlink]) + AC_REQUIRE([gl_FUNC_FACCESSAT_EOVERFLOW]) + AC_REQUIRE([gl_FUNC_LSTAT_FOLLOWS_SLASHED_SYMLINK]) + AC_CHECK_FUNCS_ONCE([canonicalize_file_name faccessat]) dnl On native Windows, we use _getcwd(), regardless whether getcwd() is dnl available through the linker option '-loldnames'. diff --git a/modules/canonicalize b/modules/canonicalize index e1b760704..29919bcdc 100644 --- a/modules/canonicalize +++ b/modules/canonicalize @@ -5,12 +5,15 @@ Files: lib/canonicalize.h lib/canonicalize.c m4/canonicalize.m4 +m4/faccessat.m4 +m4/lstat.m4 Depends-on: attribute double-slash-root errno extensions +fcntl-h file-set filename free-posix diff --git a/modules/canonicalize-lgpl b/modules/canonicalize-lgpl index e94a898bd..dfad9202f 100644 --- a/modules/canonicalize-lgpl +++ b/modules/canonicalize-lgpl @@ -5,6 +5,8 @@ Files: lib/canonicalize-lgpl.c m4/canonicalize.m4 m4/double-slash-root.m4 +m4/faccessat.m4 +m4/lstat.m4 Depends-on: extensions @@ -12,6 +14,7 @@ stdlib nocrash double-slash-root [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] errno [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] +fcntl-h [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] filename [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] free-posix [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] idx [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test $REPLACE_CANONICALIZE_FILE_NAME = 1] -- 2.27.0