[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug
From: |
Adhemerval Zanella |
Subject: |
Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug |
Date: |
Fri, 11 Dec 2020 10:03:55 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 |
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 <eggert@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 <bruno@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
>
- [PATCH 2/6] canonicalize: fix EOVERFLOW bug, (continued)
[PATCH 6/6] canonicalize: refactor can_mode flag, Paul Eggert, 2020/12/02
[PATCH 5/6] canonicalize: prefer signed integer types, Paul Eggert, 2020/12/02
Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug,
Adhemerval Zanella <=
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Paul Eggert, 2020/12/17
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Adhemerval Zanella, 2020/12/18
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Adhemerval Zanella, 2020/12/18
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Paul Eggert, 2020/12/18
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Paul Eggert, 2020/12/24
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Tom G. Christensen, 2020/12/31
- Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug, Bruno Haible, 2020/12/31