[PATCH v3 4/6] stdlib: Sync canonicalize with gnulib [BZ #10635] [BZ #26592] [BZ #26341] [BZ #24970]

[Adhemerval Zanella]

It sync with gnulib version b29d62dfa with the following change:

--
diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index 04fe95253f..c8f085b779 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -41,6 +41,7 @@
 #include <filename.h>
 #include <idx.h>
 #include <scratch_buffer.h>
+#include <intprops.h>

 #ifdef _LIBC
 # include <shlib-compat.h>
@@ -339,6 +340,11 @@ realpath_stk (const char *name, char *resolved,
               if (end_in_extra_buffer)
                 end_idx = end - extra_buf;
               idx_t len = strlen (end);
+              if (INT_ADD_OVERFLOW (len, n))
+                {
+                  __set_errno (ENAMETOOLONG);
+                  goto error_nomem;
+                }
               while (extra_buffer.length <= len + n)
                 {
                   if (!scratch_buffer_grow_preserve (&extra_buffer))
--

It is required to avoid stdlib/test-bz22786 regression, where it passes
string larger than PTRDIFF_T as the input argument.  Althought it uses
a pointer larger than the one malloc would return (BZ#23741), it is
still a semantic support for glibc and ENAMETOOLONG should be returned.

The patch also fixes multiple realpath issues:

  - Portability fixes for errno clobbering on free (BZ#10635).  The
    function does not call free directly anymore, although it might be
    done through scratch_buffer_free.  The free errno clobbering is
    being tracked by BZ#17924.

  - Pointer arithmetic overflows in realpath (BZ#26592).

  - Realpath cyclically call __alloca(path_max) to consume too much
    stack space (BZ#26341).

  - Realpath mishandles EOVERFLOW; stat not needed anyway (BZ#24970).
    The check is done through faccessat now.

Checked on x86_64-linux-gnu.
---
 stdlib/canonicalize.c               | 547 +++++++++++++++++++---------
 sysdeps/unix/sysv/linux/faccessat.c |   3 +-
 2 files changed, 386 insertions(+), 164 deletions(-)

diff --git a/stdlib/canonicalize.c b/stdlib/canonicalize.c
index 3fcb399a5d..c8f085b779 100644
--- a/stdlib/canonicalize.c
+++ b/stdlib/canonicalize.c
@@ -16,43 +16,200 @@
    License along with the GNU C Library; if not, see
    <https://www.gnu.org/licenses/>.  */
 
-#include <assert.h>
+#ifndef _LIBC
+/* Don't use __attribute__ __nonnull__ in this compilation unit.  Otherwise gcc
+   optimizes away the name == NULL test below.  */
+# define _GL_ARG_NONNULL(params)
+
+# define _GL_USE_STDLIB_ALLOC 1
+# include <libc-config.h>
+#endif
+
+/* Specification.  */
 #include <stdlib.h>
-#include <string.h>
-#include <unistd.h>
-#include <limits.h>
-#include <sys/stat.h>
+
 #include <errno.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
 #include <stddef.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
 
 #include <eloop-threshold.h>
-#include <shlib-compat.h>
+#include <filename.h>
+#include <idx.h>
+#include <scratch_buffer.h>
+#include <intprops.h>
+
+#ifdef _LIBC
+# include <shlib-compat.h>
+# define GCC_LINT 1
+# define _GL_ATTRIBUTE_PURE __attribute__ ((__pure__))
+#else
+# define __canonicalize_file_name canonicalize_file_name
+# define __realpath realpath
+# include "pathmax.h"
+# define __faccessat faccessat
+# if defined _WIN32 && !defined __CYGWIN__
+#  define __getcwd _getcwd
+# elif HAVE_GETCWD
+#  if IN_RELOCWRAPPER
+    /* When building the relocatable program wrapper, use the system's getcwd
+       function, not the gnulib override, otherwise we would get a link error.
+     */
+#   undef getcwd
+#  endif
+#  if defined VMS && !defined getcwd
+    /* We want the directory in Unix syntax, not in VMS syntax.
+       The gnulib override of 'getcwd' takes 2 arguments; the original VMS
+       'getcwd' takes 3 arguments.  */
+#   define __getcwd(buf, max) getcwd (buf, max, 0)
+#  else
+#   define __getcwd getcwd
+#  endif
+# else
+#  define __getcwd(buf, max) getwd (buf)
+# endif
+# define __mempcpy mempcpy
+# define __pathconf pathconf
+# define __rawmemchr rawmemchr
+# define __readlink readlink
+# define __stat stat
+#endif
 
-/* Return the canonical absolute name of file NAME.  A canonical name
-   does not contain any `.', `..' components nor any repeated path
-   separators ('/') or symlinks.  All path components must exist.  If
-   RESOLVED is null, the result is malloc'd; otherwise, if the
-   canonical name is PATH_MAX chars or more, returns null with `errno'
-   set to ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars,
-   returns the name in RESOLVED.  If the name cannot be resolved and
-   RESOLVED is non-NULL, it contains the path of the first component
-   that cannot be resolved.  If the path can be resolved, RESOLVED
-   holds the same value as the value returned.  */
+/* Suppress bogus GCC -Wmaybe-uninitialized warnings.  */
+#if defined GCC_LINT || defined lint
+# define IF_LINT(Code) Code
+#else
+# define IF_LINT(Code) /* empty */
+#endif
 
-char *
-__realpath (const char *name, char *resolved)
+#ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT
+# define DOUBLE_SLASH_IS_DISTINCT_ROOT false
+#endif
+
+#if defined _LIBC || !FUNC_REALPATH_WORKS
+
+/* 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
+  return __faccessat (AT_FDCWD, file, F_OK, AT_EACCESS) == 0;
+# else
+  struct stat st;
+  return __stat (file, &st) == 0 || errno == EOVERFLOW;
+# endif
+}
+
+/* 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
+   check this later anyway when it checks an ordinary file name
+   component within END.  END must either be empty, or start with a
+   slash.  */
+
+static bool _GL_ATTRIBUTE_PURE
+suffix_requires_dir_check (char const *end)
+{
+  /* If END does not start with a slash, the suffix is OK.  */
+  while (ISSLASH (*end))
+    {
+      /* Two or more slashes act like a single slash.  */
+      do
+        end++;
+      while (ISSLASH (*end));
+
+      switch (*end++)
+        {
+        default: return false;  /* An ordinary file name component is OK.  */
+        case '\0': return true; /* Trailing "/" is trouble.  */
+        case '.': break;        /* Possibly "." or "..".  */
+        }
+      /* Trailing "/.", or "/.." even if not trailing, is trouble.  */
+      if (!*end || (*end == '.' && (!end[1] || ISSLASH (end[1]))))
+        return true;
+    }
+
+  return false;
+}
+
+/* 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 <https://bugs.gnu.org/30350>, 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 .. strlen (dir_suffix)].  */
+
+static bool
+dir_check (char *dir, char *dirend)
+{
+  strcpy (dirend, dir_suffix);
+  return file_accessible (dir);
+}
+
+static idx_t
+get_path_max (void)
+{
+# ifdef PATH_MAX
+  long int path_max = PATH_MAX;
+# else
+  /* The caller invoked realpath with a null RESOLVED, even though
+     PATH_MAX is not defined as a constant.  The glibc manual says
+     programs should not do this, and POSIX says the behavior is undefined.
+     Historically, glibc here used the result of pathconf, or 1024 if that
+     failed; stay consistent with this (dubious) historical practice.  */
+  int err = errno;
+  long int path_max = __pathconf ("/", _PC_PATH_MAX);
+  __set_errno (err);
+# endif
+  return path_max < 0 ? 1024 : path_max <= IDX_MAX ? path_max : IDX_MAX;
+}
+
+/* Act like __realpath (see below), with an additional argument
+   rname_buf that can be used as temporary storage.
+
+   If GCC_LINT is defined, do not inline this function with GCC 10.1
+   and later, to avoid creating a pointer to the stack that GCC
+   -Wreturn-local-addr incorrectly complains about.  See:
+   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644
+   Although the noinline attribute can hurt performance a bit, no better way
+   to pacify GCC is known; even an explicit #pragma does not pacify GCC.
+   When the GCC bug is fixed this workaround should be limited to the
+   broken GCC versions.  */
+#if __GNUC_PREREQ (10, 1)
+# if defined GCC_LINT || defined lint
+__attribute__ ((__noinline__))
+# elif __OPTIMIZE__ && !__NO_INLINE__
+#  define GCC_BOGUS_WRETURN_LOCAL_ADDR
+# endif
+#endif
+static char *
+realpath_stk (const char *name, char *resolved,
+              struct scratch_buffer *rname_buf)
 {
-  char *rpath, *dest, *extra_buf = NULL;
-  const char *start, *end, *rpath_limit;
-  long int path_max;
+  char *dest;
+  char const *start;
+  char const *end;
   int num_links = 0;
 
   if (name == NULL)
     {
       /* As per Single Unix Specification V2 we must return an error if
-        either parameter is a null pointer.  We extend this to allow
-        the RESOLVED parameter to be NULL in case the we are expected to
-        allocate the room for the return value.  */
+         either parameter is a null pointer.  We extend this to allow
+         the RESOLVED parameter to be NULL in case the we are expected to
+         allocate the room for the return value.  */
       __set_errno (EINVAL);
       return NULL;
     }
@@ -60,166 +217,230 @@ __realpath (const char *name, char *resolved)
   if (name[0] == '\0')
     {
       /* As per Single Unix Specification V2 we must return an error if
-        the name argument points to an empty string.  */
+         the name argument points to an empty string.  */
       __set_errno (ENOENT);
       return NULL;
     }
 
-#ifdef PATH_MAX
-  path_max = PATH_MAX;
-#else
-  path_max = __pathconf (name, _PC_PATH_MAX);
-  if (path_max <= 0)
-    path_max = 1024;
-#endif
+  struct scratch_buffer extra_buffer, link_buffer;
+  scratch_buffer_init (&extra_buffer);
+  scratch_buffer_init (&link_buffer);
+  scratch_buffer_init (rname_buf);
+  char *rname_on_stack = rname_buf->data;
+  char *rname = rname_on_stack;
+  bool end_in_extra_buffer = false;
+  bool failed = true;
 
-  if (resolved == NULL)
-    {
-      rpath = malloc (path_max);
-      if (rpath == NULL)
-       return NULL;
-    }
-  else
-    rpath = resolved;
-  rpath_limit = rpath + path_max;
+  /* This is always zero for Posix hosts, but can be 2 for MS-Windows
+     and MS-DOS X:/foo/bar file names.  */
+  idx_t prefix_len = FILE_SYSTEM_PREFIX_LEN (name);
 
-  if (name[0] != '/')
+  if (!IS_ABSOLUTE_FILE_NAME (name))
     {
-      if (!__getcwd (rpath, path_max))
-       {
-         rpath[0] = '\0';
-         goto error;
-       }
-      dest = __rawmemchr (rpath, '\0');
+      while (!__getcwd (rname, rname_buf->length))
+        {
+          if (errno != ERANGE)
+            {
+              dest = rname;
+              goto error;
+            }
+          if (!scratch_buffer_grow (rname_buf))
+            goto error_nomem;
+          rname = rname_buf->data;
+        }
+      dest = __rawmemchr (rname, '\0');
+      start = name;
+      prefix_len = FILE_SYSTEM_PREFIX_LEN (rname);
     }
   else
     {
-      rpath[0] = '/';
-      dest = rpath + 1;
+      dest = __mempcpy (rname, name, prefix_len);
+      *dest++ = '/';
+      if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+        {
+          if (prefix_len == 0 /* implies ISSLASH (name[0]) */
+              && ISSLASH (name[1]) && !ISSLASH (name[2]))
+            *dest++ = '/';
+          *dest = '\0';
+        }
+      start = name + prefix_len;
     }
 
-  for (start = end = name; *start; start = end)
+  for ( ; *start; start = end)
     {
-      struct stat64 st;
-      int n;
-
-      /* Skip sequence of multiple path-separators.  */
-      while (*start == '/')
-       ++start;
-
-      /* Find end of path component.  */
-      for (end = start; *end && *end != '/'; ++end)
-       /* Nothing.  */;
-
-      if (end - start == 0)
-       break;
-      else if (end - start == 1 && start[0] == '.')
-       /* nothing */;
-      else if (end - start == 2 && start[0] == '.' && start[1] == '.')
-       {
-         /* Back up to previous component, ignore if at root already.  */
-         if (dest > rpath + 1)
-           while ((--dest)[-1] != '/');
-       }
+      /* Skip sequence of multiple file name separators.  */
+      while (ISSLASH (*start))
+        ++start;
+
+      /* Find end of component.  */
+      for (end = start; *end && !ISSLASH (*end); ++end)
+        /* Nothing.  */;
+
+      /* Length of this file name component; it can be zero if a file
+         name ends in '/'.  */
+      idx_t startlen = end - start;
+
+      if (startlen == 0)
+        break;
+      else if (startlen == 1 && start[0] == '.')
+        /* nothing */;
+      else if (startlen == 2 && start[0] == '.' && start[1] == '.')
+        {
+          /* Back up to previous component, ignore if at root already.  */
+          if (dest > rname + prefix_len + 1)
+            for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest)
+              continue;
+          if (DOUBLE_SLASH_IS_DISTINCT_ROOT
+              && dest == rname + 1 && !prefix_len
+              && ISSLASH (*dest) && !ISSLASH (dest[1]))
+            dest++;
+        }
       else
-       {
-         size_t new_size;
-
-         if (dest[-1] != '/')
-           *dest++ = '/';
-
-         if (dest + (end - start) >= rpath_limit)
-           {
-             ptrdiff_t dest_offset = dest - rpath;
-             char *new_rpath;
-
-             if (resolved)
-               {
-                 __set_errno (ENAMETOOLONG);
-                 if (dest > rpath + 1)
-                   dest--;
-                 *dest = '\0';
-                 goto error;
-               }
-             new_size = rpath_limit - rpath;
-             if (end - start + 1 > path_max)
-               new_size += end - start + 1;
-             else
-               new_size += path_max;
-             new_rpath = (char *) realloc (rpath, new_size);
-             if (new_rpath == NULL)
-               goto error;
-             rpath = new_rpath;
-             rpath_limit = rpath + new_size;
-
-             dest = rpath + dest_offset;
-           }
-
-         dest = __mempcpy (dest, start, end - start);
-         *dest = '\0';
-
-         if (__lstat64 (rpath, &st) < 0)
-           goto error;
-
-         if (S_ISLNK (st.st_mode))
-           {
-             char *buf = __alloca (path_max);
-             size_t len;
-
-             if (++num_links > __eloop_threshold ())
-               {
-                 __set_errno (ELOOP);
-                 goto error;
-               }
-
-             n = __readlink (rpath, buf, path_max - 1);
-             if (n < 0)
-               goto error;
-             buf[n] = '\0';
-
-             if (!extra_buf)
-               extra_buf = __alloca (path_max);
-
-             len = strlen (end);
-             if (path_max - n <= len)
-               {
-                 __set_errno (ENAMETOOLONG);
-                 goto error;
-               }
-
-             /* Careful here, end may be a pointer into extra_buf... */
-             memmove (&extra_buf[n], end, len + 1);
-             name = end = memcpy (extra_buf, buf, n);
-
-             if (buf[0] == '/')
-               dest = rpath + 1;       /* It's an absolute symlink */
-             else
-               /* Back up to previous component, ignore if at root already: */
-               if (dest > rpath + 1)
-                 while ((--dest)[-1] != '/');
-           }
-         else if (!S_ISDIR (st.st_mode) && *end != '\0')
-           {
-             __set_errno (ENOTDIR);
-             goto error;
-           }
-       }
+        {
+          if (!ISSLASH (dest[-1]))
+            *dest++ = '/';
+
+          while (rname + rname_buf->length - dest
+                 < startlen + sizeof dir_suffix)
+            {
+              idx_t dest_offset = dest - rname;
+              if (!scratch_buffer_grow_preserve (rname_buf))
+                goto error_nomem;
+              rname = rname_buf->data;
+              dest = rname + dest_offset;
+            }
+
+          dest = __mempcpy (dest, start, startlen);
+          *dest = '\0';
+
+          char *buf;
+          ssize_t n;
+          while (true)
+            {
+              buf = link_buffer.data;
+              idx_t bufsize = link_buffer.length;
+              n = __readlink (rname, buf, bufsize - 1);
+              if (n < bufsize - 1)
+                break;
+              if (!scratch_buffer_grow (&link_buffer))
+                goto error_nomem;
+            }
+          if (0 <= n)
+            {
+              if (++num_links > __eloop_threshold ())
+                {
+                  __set_errno (ELOOP);
+                  goto error;
+                }
+
+              buf[n] = '\0';
+
+              char *extra_buf = extra_buffer.data;
+              idx_t end_idx IF_LINT (= 0);
+              if (end_in_extra_buffer)
+                end_idx = end - extra_buf;
+              idx_t len = strlen (end);
+              if (INT_ADD_OVERFLOW (len, n))
+                {
+                  __set_errno (ENAMETOOLONG);
+                  goto error_nomem;
+                }
+              while (extra_buffer.length <= len + n)
+                {
+                  if (!scratch_buffer_grow_preserve (&extra_buffer))
+                    goto error_nomem;
+                  extra_buf = extra_buffer.data;
+                }
+              if (end_in_extra_buffer)
+                end = extra_buf + end_idx;
+
+              /* Careful here, end may be a pointer into extra_buf... */
+              memmove (&extra_buf[n], end, len + 1);
+              name = end = memcpy (extra_buf, buf, n);
+              end_in_extra_buffer = true;
+
+              if (IS_ABSOLUTE_FILE_NAME (buf))
+                {
+                  idx_t pfxlen = FILE_SYSTEM_PREFIX_LEN (buf);
+
+                  dest = __mempcpy (rname, buf, pfxlen);
+                  *dest++ = '/'; /* It's an absolute symlink */
+                  if (DOUBLE_SLASH_IS_DISTINCT_ROOT)
+                    {
+                      if (ISSLASH (buf[1]) && !ISSLASH (buf[2]) && !pfxlen)
+                        *dest++ = '/';
+                      *dest = '\0';
+                    }
+                  /* Install the new prefix to be in effect hereafter.  */
+                  prefix_len = pfxlen;
+                }
+              else
+                {
+                  /* Back up to previous component, ignore if at root
+                     already: */
+                  if (dest > rname + prefix_len + 1)
+                    for (--dest; dest > rname && !ISSLASH (dest[-1]); --dest)
+                      continue;
+                  if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1
+                      && ISSLASH (*dest) && !ISSLASH (dest[1]) && !prefix_len)
+                    dest++;
+                }
+            }
+          else if (! (suffix_requires_dir_check (end)
+                      ? dir_check (rname, dest)
+                      : errno == EINVAL))
+            goto error;
+        }
     }
-  if (dest > rpath + 1 && dest[-1] == '/')
+  if (dest > rname + prefix_len + 1 && ISSLASH (dest[-1]))
     --dest;
-  *dest = '\0';
-
-  assert (resolved == NULL || resolved == rpath);
-  return rpath;
+  if (DOUBLE_SLASH_IS_DISTINCT_ROOT && dest == rname + 1 && !prefix_len
+      && ISSLASH (*dest) && !ISSLASH (dest[1]))
+    dest++;
+  failed = false;
 
 error:
-  assert (resolved == NULL || resolved == rpath);
-  if (resolved == NULL)
-    free (rpath);
-  return NULL;
+  *dest++ = '\0';
+  if (resolved != NULL && dest - rname <= get_path_max ())
+    rname = strcpy (resolved, rname);
+
+error_nomem:
+  scratch_buffer_free (&extra_buffer);
+  scratch_buffer_free (&link_buffer);
+
+  if (failed || rname == resolved)
+    {
+      scratch_buffer_free (rname_buf);
+      return failed ? NULL : resolved;
+    }
+
+  return scratch_buffer_dupfree (rname_buf, dest - rname);
+}
+
+/* Return the canonical absolute name of file NAME.  A canonical name
+   does not contain any ".", ".." components nor any repeated file name
+   separators ('/') or symlinks.  All file name components must exist.  If
+   RESOLVED is null, the result is malloc'd; otherwise, if the
+   canonical name is PATH_MAX chars or more, returns null with 'errno'
+   set to ENAMETOOLONG; if the name fits in fewer than PATH_MAX chars,
+   returns the name in RESOLVED.  If the name cannot be resolved and
+   RESOLVED is non-NULL, it contains the name of the first component
+   that cannot be resolved.  If the name can be resolved, RESOLVED
+   holds the same value as the value returned.  */
+
+char *
+__realpath (const char *name, char *resolved)
+{
+  #ifdef GCC_BOGUS_WRETURN_LOCAL_ADDR
+   #warning "GCC might issue a bogus -Wreturn-local-addr warning here."
+   #warning "See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=93644>."
+  #endif
+  struct scratch_buffer rname_buffer;
+  return realpath_stk (name, resolved, &rname_buffer);
 }
 libc_hidden_def (__realpath)
 versioned_symbol (libc, __realpath, realpath, GLIBC_2_3);
+#endif /* !FUNC_REALPATH_WORKS || defined _LIBC */
 
 
 #if SHLIB_COMPAT(libc, GLIBC_2_0, GLIBC_2_3)
diff --git a/sysdeps/unix/sysv/linux/faccessat.c 
b/sysdeps/unix/sysv/linux/faccessat.c
index 5d078371b5..5bb1051c06 100644
--- a/sysdeps/unix/sysv/linux/faccessat.c
+++ b/sysdeps/unix/sysv/linux/faccessat.c
@@ -24,7 +24,7 @@
 
 
 int
-faccessat (int fd, const char *file, int mode, int flag)
+__faccessat (int fd, const char *file, int mode, int flag)
 {
   int ret = INLINE_SYSCALL_CALL (faccessat2, fd, file, mode, flag);
 #if __ASSUME_FACCESSAT2
@@ -73,3 +73,4 @@ faccessat (int fd, const char *file, int mode, int flag)
   return INLINE_SYSCALL_ERROR_RETURN_VALUE (EACCES);
 #endif /* !__ASSUME_FACCESSAT2 */
 }
+weak_alias (__faccessat, faccessat)
-- 
2.25.1