[v5] io: Fix ftw internal realloc buffer (BZ #28126)

Message ID 20210907123352.132575-1-adhemerval.zanella@linaro.org
State Superseded
Delegated to: Siddhesh Poyarekar
Headers
Series [v5] io: Fix ftw internal realloc buffer (BZ #28126) |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit fail Patch caused testsuite regressions

Commit Message

Adhemerval Zanella Sept. 7, 2021, 12:33 p.m. UTC
  The 106ff08526d3ca did not take in consideration the buffer might be
reallocated if the total path is larger than PATH_MAX.  The realloc
uses 'dirbuf', where 'dirstreams' is the allocated buffer.

Checked on x86_64-linux-gnu.
---
Changes in v5:
 - Fail as unsupported if the large path can not be created, so it
   do not regress on filesystem that do no support it (such as
   overlayfs used on buildbot).

---
 io/Makefile          |  1 +
 io/ftw.c             | 13 ++++---
 io/tst-ftw-bz28126.c | 92 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 io/tst-ftw-bz28126.c
  

Comments

Siddhesh Poyarekar Sept. 13, 2021, 1:15 p.m. UTC | #1
On 9/7/21 6:03 PM, Adhemerval Zanella via Libc-alpha wrote:
> The 106ff08526d3ca did not take in consideration the buffer might be
> reallocated if the total path is larger than PATH_MAX.  The realloc
> uses 'dirbuf', where 'dirstreams' is the allocated buffer.
> 
> Checked on x86_64-linux-gnu.

The new test seems to be failing on i686:

https://www.delorie.com/trybots/32bit/2760/io-tst-ftw-bz28126.out

Siddhesh
  

Patch

diff --git a/io/Makefile b/io/Makefile
index 9871ecbc74..ecf65aba60 100644
--- a/io/Makefile
+++ b/io/Makefile
@@ -79,6 +79,7 @@  tests		:= test-utime test-stat test-stat2 test-lfs tst-getcwd \
 		   tst-futimens \
 		   tst-utimensat \
 		   tst-closefrom \
+		   tst-ftw-bz28126
 
 tests-time64 := \
   tst-futimens-time64 \
diff --git a/io/ftw.c b/io/ftw.c
index f0db173727..3964c07bbb 100644
--- a/io/ftw.c
+++ b/io/ftw.c
@@ -391,13 +391,16 @@  process_entry (struct ftw_data *data, struct dir_data *dir, const char *name,
   if (data->dirbufsize < new_buflen)
     {
       /* Enlarge the buffer.  */
-      char *newp;
-
-      data->dirbufsize = 2 * new_buflen;
-      newp = (char *) realloc (data->dirbuf, data->dirbufsize);
+      size_t newsize = 2 * new_buflen;
+      void *newp = realloc (data->dirstreams, data->maxdir
+					      * sizeof (struct dir_data *)
+					      + newsize);
       if (newp == NULL)
 	return -1;
-      data->dirbuf = newp;
+      data->dirstreams = newp;
+      data->dirbufsize = newsize;
+      data->dirbuf = (char *) data->dirstreams
+		     + data->maxdir * sizeof (struct dir_data *);
     }
 
   *((char *) __mempcpy (data->dirbuf + data->ftw.base, name, namlen)) = '\0';
diff --git a/io/tst-ftw-bz28126.c b/io/tst-ftw-bz28126.c
new file mode 100644
index 0000000000..565cd92cfd
--- /dev/null
+++ b/io/tst-ftw-bz28126.c
@@ -0,0 +1,92 @@ 
+/* Check if internal buffer reallocation work for large paths (BZ #28126)
+   Copyright (C) 2021 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
+   <https://www.gnu.org/licenses/>.  */
+
+#include <errno.h>
+#include <ftw.h>
+#include <limits.h>
+#include <string.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/support.h>
+#include <support/temp_file.h>
+#include <support/xunistd.h>
+#include <stdio.h>
+
+static int
+my_func (const char *file, const struct stat *sb, int flag)
+{
+  return 0;
+}
+
+static const char folder[NAME_MAX] = { [0 ... 253] = 'a', [254] = '\0' };
+
+#define NSUBFOLDERS 16
+static int nsubfolders;
+
+static void
+do_cleanup (void)
+{
+  xchdir ("..");
+  for (int i = 0; i < nsubfolders; i++)
+    {
+      remove (folder);
+      xchdir ("..");
+    }
+  remove (folder);
+}
+#define CLEANUP_HANDLER do_cleanup
+
+static int
+do_test (void)
+{
+  char *tempdir = support_create_temp_directory ("tst-bz28126");
+
+  /* Create path with various subfolders to force an internal buffer
+     reallocation within ntfw.  */
+  char *path = xasprintf ("%s/%s", tempdir, folder);
+  xmkdir (path, 0777);
+  xchdir (path);
+  free (path);
+  for (int i = 0; i < NSUBFOLDERS - 1; i++)
+    {
+      int r = mkdir (folder, 0777);
+      /* Some filesystem such as overlayfs does not support larger
+	 path required to trigger the internal buffer reallocation.  */
+      if (r != 0)
+	{
+	  if (errno == ENAMETOOLONG)
+	    FAIL_UNSUPPORTED ("the filesystem does not support the required"
+			      "large path");
+	  else
+	    FAIL_EXIT1 ("mkdir (\"%s\", 0%o): %m", folder, 0777);
+	}
+
+      xchdir (folder);
+      nsubfolders++;
+    }
+
+  TEST_COMPARE (ftw (tempdir, my_func, 20), 0);
+
+  free (tempdir);
+
+  do_cleanup ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>