[PATCH] glob: Avoid copying the d_name field of struct dirent [BZ #19779]

[Florian Weimer <fweimer at redhat dot com>]

Introducing a new struct makes it unnecessary to copy the name out of
the dirent structure.   The name is subsequently allocated on the heap
anyway.

I extended the existing test with a long name.  The test now crashes
before the fix.

We should split sysdeps/unix/sysv/linux/i386/glob64.c into multiple
files, rather than including posix/glob.c multiple times.

DIRENT_MUST_BE can be removed, but that's an unrelated cleanup (it was
dead before).

Florian

2016-03-11  Florian Weimer  <fweimer@redhat.com>

	[BZ #19779]
	Avoid copying names of directory entries.
	* posix/glob.c (NAMELEN, CONVERT_D_NAMLEN, CONVERT_D_INO)
	(CONVERT_D_TYPE CONVERT_DIRENT_DIRENT64, REAL_DIR_ENTRY)
	(NAME_MAX): Remove macros.
	(struct abstract_dirent): New type.
	(convert_dirent): New function.
	(glob_in_dir): Use struct abstract_dirent.  Call convert_dirent.
	Adjust references to the dirent object.  Use strdup instead of
	NAMELEN and mempcpy.
	* sysdeps/gnu/glob64.c (COMPILE_GLOB64): Remove macro.
	* sysdeps/unix/sysv/linux/i386/glob64.c (COMPILE_GLOB64): Likewise.
	(convert_dirent): Redefine for second file inclusion.
	* posix/bug-glob2.c (LONG_NAME): Define.
	(filesystem): Add LONG_NAME.
	(my_DIR): Increase the size of room_for_dirent.

diff --git a/posix/bug-glob2.c b/posix/bug-glob2.c
index ddf5ec9..581f28e 100644
--- a/posix/bug-glob2.c
+++ b/posix/bug-glob2.c
@@ -40,6 +40,17 @@
 # define PRINTF(fmt, args...)
 #endif
 
+#define LONG_NAME \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx" \
+  "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
 
 static struct
 {
@@ -58,6 +69,7 @@ static struct
       { ".", 3, DT_DIR, 0755 },
       { "..", 3, DT_DIR, 0755 },
       { "a", 3, DT_REG, 0644 },
+      { LONG_NAME, 3, DT_REG, 0644 },
     { "unreadable", 2, DT_DIR, 0111 },
       { ".", 3, DT_DIR, 0111 },
       { "..", 3, DT_DIR, 0755 },
@@ -75,7 +87,7 @@ typedef struct
   int level;
   int idx;
   struct dirent d;
-  char room_for_dirent[NAME_MAX];
+  char room_for_dirent[1000];
 } my_DIR;
 
 
diff --git a/posix/glob.c b/posix/glob.c
index 0c04c3c..aea1b71 100644
--- a/posix/glob.c
+++ b/posix/glob.c
@@ -24,6 +24,7 @@
 #include <errno.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <stdbool.h>
 #include <stddef.h>
 
 /* Outcomment the following line for production quality code.  */
@@ -57,10 +58,8 @@
 
 #if defined HAVE_DIRENT_H || defined __GNU_LIBRARY__
 # include <dirent.h>
-# define NAMLEN(dirent) strlen((dirent)->d_name)
 #else
 # define dirent direct
-# define NAMLEN(dirent) (dirent)->d_namlen
 # ifdef HAVE_SYS_NDIR_H
 #  include <sys/ndir.h>
 # endif
@@ -75,13 +74,6 @@
 # endif /* HAVE_VMSDIR_H */
 #endif
 
-
-/* In GNU systems, <dirent.h> defines this macro for us.  */
-#ifdef _D_NAMLEN
-# undef NAMLEN
-# define NAMLEN(d) _D_NAMLEN(d)
-#endif
-
 /* When used in the GNU libc the symbol _DIRENT_HAVE_D_TYPE is available
    if the `d_type' member for `struct dirent' is available.
    HAVE_STRUCT_DIRENT_D_TYPE plays the same role in GNULIB.  */
@@ -103,54 +95,8 @@
 # define DIRENT_MIGHT_BE_DIR(d)		true
 #endif /* HAVE_D_TYPE */
 
-/* If the system has the `struct dirent64' type we use it internally.  */
-#if defined _LIBC && !defined COMPILE_GLOB64
-# if defined HAVE_DIRENT_H || defined __GNU_LIBRARY__
-#  define CONVERT_D_NAMLEN(d64, d32)
-# else
-#  define CONVERT_D_NAMLEN(d64, d32) \
-  (d64)->d_namlen = (d32)->d_namlen;
-# endif
-
-# if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__
-#  define CONVERT_D_INO(d64, d32)
-# else
-#  define CONVERT_D_INO(d64, d32) \
-  (d64)->d_ino = (d32)->d_ino;
-# endif
-
-# ifdef _DIRENT_HAVE_D_TYPE
-#  define CONVERT_D_TYPE(d64, d32) \
-  (d64)->d_type = (d32)->d_type;
-# else
-#  define CONVERT_D_TYPE(d64, d32)
-# endif
-
-# define CONVERT_DIRENT_DIRENT64(d64, d32) \
-  memcpy ((d64)->d_name, (d32)->d_name, NAMLEN (d32) + 1);		      \
-  CONVERT_D_NAMLEN (d64, d32)						      \
-  CONVERT_D_INO (d64, d32)						      \
-  CONVERT_D_TYPE (d64, d32)
-#endif
-
-
-#if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__
-/* Posix does not require that the d_ino field be present, and some
-   systems do not provide it. */
-# define REAL_DIR_ENTRY(dp) 1
-#else
-# define REAL_DIR_ENTRY(dp) (dp->d_ino != 0)
-#endif /* POSIX */
-
 #include <stdlib.h>
 #include <string.h>
-
-/* NAME_MAX is usually defined in <dirent.h> or <limits.h>.  */
-#include <limits.h>
-#ifndef NAME_MAX
-# define NAME_MAX (sizeof (((struct dirent *) 0)->d_name))
-#endif
-
 #include <alloca.h>
 
 #ifdef _LIBC
@@ -195,8 +141,37 @@
 
 static const char *next_brace_sub (const char *begin, int flags) __THROWNL;
 
+/* A representation of a directory entry which does not depend on the
+   layout of struct dirent, or the size of ino_t.  */
+struct abstract_dirent
+{
+  const char *d_name;
+  int d_type;
+  bool skip_entry;
+};
+
 #endif /* !defined _LIBC || !defined GLOB_ONLY_P */
 
+/* Extract name and type from directory entry.  No copy of the name is
+   made.  */
+static void
+convert_dirent (const struct dirent *source, struct abstract_dirent *target)
+{
+  target->d_name = source->d_name;
+#if defined _DIRENT_HAVE_D_TYPE || defined HAVE_STRUCT_DIRENT_D_TYPE
+  target->d_type = source->d_type;
+#else
+  target->d_type = 0;
+#endif
+#if (defined POSIX || defined WINDOWS32) && !defined __GNU_LIBRARY__
+  /* Posix does not require that the d_ino field be present, and some
+     systems do not provide it. */
+  target->skip_entry = false;
+#else
+  target->skip_entry = source->d_ino == 0;
+#endif
+}
+
 #ifndef attribute_hidden
 # define attribute_hidden
 #endif
@@ -1553,56 +1528,31 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 
 	  while (1)
 	    {
-	      const char *name;
-	      size_t len;
-#if defined _LIBC && !defined COMPILE_GLOB64
-	      struct dirent64 *d;
-	      union
-		{
-		  struct dirent64 d64;
-		  char room [offsetof (struct dirent64, d_name[0])
-			     + NAME_MAX + 1];
-		}
-	      d64buf;
-
-	      if (__glibc_unlikely (flags & GLOB_ALTDIRFUNC))
-		{
-		  struct dirent *d32 = (*pglob->gl_readdir) (stream);
-		  if (d32 != NULL)
-		    {
-		      CONVERT_DIRENT_DIRENT64 (&d64buf.d64, d32);
-		      d = &d64buf.d64;
-		    }
-		  else
-		    d = NULL;
-		}
-	      else
-		d = __readdir64 (stream);
-#else
-	      struct dirent *d = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
-				  ? ((struct dirent *)
-				     (*pglob->gl_readdir) (stream))
-				  : __readdir (stream));
-#endif
-	      if (d == NULL)
-		break;
-	      if (! REAL_DIR_ENTRY (d))
+	      struct abstract_dirent e;
+	      {
+		struct dirent *d = (__builtin_expect (flags & GLOB_ALTDIRFUNC, 0)
+				    ? ((struct dirent *)
+				       (*pglob->gl_readdir) (stream))
+				    : __readdir (stream));
+		if (d == NULL)
+		  break;
+		convert_dirent (d, &e);
+	      }
+	      if (e.skip_entry)
 		continue;
 
 	      /* If we shall match only directories use the information
 		 provided by the dirent call if possible.  */
-	      if ((flags & GLOB_ONLYDIR) && !DIRENT_MIGHT_BE_DIR (d))
+	      if ((flags & GLOB_ONLYDIR) && !DIRENT_MIGHT_BE_DIR (&e))
 		continue;
 
-	      name = d->d_name;
-
-	      if (fnmatch (pattern, name, fnm_flags) == 0)
+	      if (fnmatch (pattern, e.d_name, fnm_flags) == 0)
 		{
 		  /* If the file we found is a symlink we have to
 		     make sure the target file exists.  */
-		  if (!DIRENT_MIGHT_BE_SYMLINK (d)
-		      || link_exists_p (dfd, directory, dirlen, name, pglob,
-					flags))
+		  if (!DIRENT_MIGHT_BE_SYMLINK (&e)
+		      || link_exists_p (dfd, directory, dirlen, e.d_name,
+					pglob, flags))
 		    {
 		      if (cur == names->count)
 			{
@@ -1622,12 +1572,10 @@ glob_in_dir (const char *pattern, const char *directory, int flags,
 			  names = newnames;
 			  cur = 0;
 			}
-		      len = NAMLEN (d);
-		      names->name[cur] = (char *) malloc (len + 1);
+		      names->name[cur] = strdup (e.d_name);
 		      if (names->name[cur] == NULL)
 			goto memory_error;
-		      *((char *) mempcpy (names->name[cur++], name, len))
-			= '\0';
+		      ++cur;
 		      ++nfound;
 		    }
 		}
diff --git a/sysdeps/gnu/glob64.c b/sysdeps/gnu/glob64.c
index d1e4e6f..d53d16d 100644
--- a/sysdeps/gnu/glob64.c
+++ b/sysdeps/gnu/glob64.c
@@ -1,3 +1,20 @@
+/* Copyright (C) 1998-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
 #include <dirent.h>
 #include <glob.h>
 #include <sys/stat.h>
@@ -17,8 +34,6 @@
 
 #define NO_GLOB_PATTERN_P 1
 
-#define COMPILE_GLOB64	1
-
 #include <posix/glob.c>
 
 libc_hidden_def (glob64)
diff --git a/sysdeps/unix/sysv/linux/i386/glob64.c b/sysdeps/unix/sysv/linux/i386/glob64.c
index b4fcd1a..727a468 100644
--- a/sysdeps/unix/sysv/linux/i386/glob64.c
+++ b/sysdeps/unix/sysv/linux/i386/glob64.c
@@ -1,3 +1,20 @@
+/* Copyright (C) 1998-2016 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
 #include <dirent.h>
 #include <glob.h>
 #include <sys/stat.h>
@@ -17,8 +34,6 @@
 
 #define NO_GLOB_PATTERN_P 1
 
-#define COMPILE_GLOB64	1
-
 #include <posix/glob.c>
 
 #include "shlib-compat.h"
@@ -43,6 +58,7 @@ int __old_glob64 (const char *__pattern, int __flags,
 #undef glob
 #define glob(pattern, flags, errfunc, pglob) \
   __old_glob64 (pattern, flags, errfunc, pglob)
+#define convert_dirent __old_convert_dirent
 #define glob_in_dir __old_glob_in_dir
 #define GLOB_ATTRIBUTE attribute_compat_text_section