On 02/12/2020 19:39, Paul Eggert wrote: > * lib/canonicalize-lgpl.c: Do not include <sys/stat.h>. > (__realpath): Do not use lstat. Just use readlink, as this > suffices and it avoids the EOVERFLOW problem that lstat has. > * modules/canonicalize-lgpl (Depends-on): Remove lstat, sys_stat.
I have sent a similar fix to reviews for this very issue for glibc (which is based on the canonicalize-lgpl) along with other fixes that you might take a look at [1]. As I hinted on glibc BZ#24970 [2], your fix does not really address all glibc regression tests. It still fails with same 2 tests I described in comment 2. I think if we could remove the alloca usage I might try to sync with gnulib version glibc code (I really want to fix BZ#26341 and also remove all possible alloca usage within glibc). [1] https://patchwork.sourceware.org/project/glibc/list/?series=1062 [2] https://sourceware.org/bugzilla/show_bug.cgi?id=24970 [3] https://sourceware.org/bugzilla/show_bug.cgi?id=26341 > --- > ChangeLog | 8 +++++++ > lib/canonicalize-lgpl.c | 49 ++++++++++++++------------------------- > modules/canonicalize-lgpl | 2 -- > 3 files changed, 26 insertions(+), 33 deletions(-) > > diff --git a/ChangeLog b/ChangeLog > index cbe4558d5..27afc2b4a 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,11 @@ > +2020-12-02 Paul Eggert <egg...@cs.ucla.edu> > + > + canonicalize-lgpl: fix EOVERFLOW bug > + * lib/canonicalize-lgpl.c: Do not include <sys/stat.h>. > + (__realpath): Do not use lstat. Just use readlink, as this > + suffices and it avoids the EOVERFLOW problem that lstat has. > + * modules/canonicalize-lgpl (Depends-on): Remove lstat, sys_stat. > + > 2020-12-02 Bruno Haible <br...@clisp.org> > > strsignal-tests: Fix test failure on macOS 10.13. > diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c > index edac98f83..981300aa7 100644 > --- a/lib/canonicalize-lgpl.c > +++ b/lib/canonicalize-lgpl.c > @@ -36,7 +36,6 @@ > #if HAVE_SYS_PARAM_H || defined _LIBC > # include <sys/param.h> > #endif > -#include <sys/stat.h> > #include <errno.h> > #include <stddef.h> > > @@ -198,12 +197,6 @@ __realpath (const char *name, char *resolved) > > for (end = start; *start; start = end) > { > -#ifdef _LIBC > - struct stat64 st; > -#else > - struct stat st; > -#endif > - > /* Skip sequence of multiple path-separators. */ > while (ISSLASH (*start)) > ++start; > @@ -212,9 +205,7 @@ __realpath (const char *name, char *resolved) > for (end = start; *end && !ISSLASH (*end); ++end) > /* Nothing. */; > > - if (end - start == 0) > - break; > - else if (end - start == 1 && start[0] == '.') > + if (end - start == 1 && start[0] == '.') > /* nothing */; > else if (end - start == 2 && start[0] == '.' && start[1] == '.') > { > @@ -272,20 +263,17 @@ __realpath (const char *name, char *resolved) > #endif > *dest = '\0'; > > - /* FIXME: if lstat fails with errno == EOVERFLOW, > - the entry exists. */ > -#ifdef _LIBC > - if (__lxstat64 (_STAT_VER, rpath, &st) < 0) > -#else > - if (lstat (rpath, &st) < 0) > -#endif > - goto error; > - > - if (S_ISLNK (st.st_mode)) > + char linkbuf[128]; > + ssize_t n = __readlink (rpath, linkbuf, sizeof linkbuf); > + if (n < 0) > + { > + if (errno != EINVAL) > + goto error; > + } > + else > { > char *buf; > size_t len; > - ssize_t n; > > if (++num_links > MAXSYMLINKS) > { > @@ -302,11 +290,15 @@ __realpath (const char *name, char *resolved) > goto error; > } > } > - buf = extra_buf + path_max; > - > - n = __readlink (rpath, buf, path_max - 1); > - if (n < 0) > - goto error; > + if (n < sizeof linkbuf) > + buf = linkbuf; > + else > + { > + buf = extra_buf + path_max; > + n = __readlink (rpath, buf, path_max - 1); > + if (n < 0) > + goto error; > + } > buf[n] = '\0'; > > len = strlen (end); > @@ -350,11 +342,6 @@ __realpath (const char *name, char *resolved) > dest++; > } > } > - else if (!S_ISDIR (st.st_mode) && *end != '\0') > - { > - __set_errno (ENOTDIR); > - goto error; > - } > } > } > if (dest > rpath + prefix_len + 1 && ISSLASH (dest[-1])) > diff --git a/modules/canonicalize-lgpl b/modules/canonicalize-lgpl > index 20ee7908b..701b492db 100644 > --- a/modules/canonicalize-lgpl > +++ b/modules/canonicalize-lgpl > @@ -14,12 +14,10 @@ alloca-opt [test $HAVE_CANONICALIZE_FILE_NAME = 0 > || test $REPLACE_CANONI > 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] > filename [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > -lstat [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > malloca [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > memmove [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > pathmax [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > readlink [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > -sys_stat [test $HAVE_CANONICALIZE_FILE_NAME = 0 || test > $REPLACE_CANONICALIZE_FILE_NAME = 1] > > configure.ac: > gl_CANONICALIZE_LGPL >
