diff mbox series

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

Message ID 20210826122230.3772402-1-adhemerval.zanella@linaro.org
State Superseded
Headers
Series [v3] 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 Aug. 26, 2021, 12:22 p.m. UTC 
  Third time's a charm... D.J note that buildbot accused a failure [1]
where mkdir failed to create the directory.  I am not sure how this
happened, I couldn't reproduce it on ext4 FS neither on tmpfs.

[1] https://www.delorie.com/trybots/32bit/2720/io-tst-ftw-bz28126.out

---

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.
---
 io/Makefile          |  1 +
 io/ftw.c             | 13 +++++---
 io/tst-ftw-bz28126.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 5 deletions(-)
 create mode 100644 io/tst-ftw-bz28126.c

Comments

Andreas Schwab Aug. 26, 2021, 1:10 p.m. UTC | #1 
On Aug 26 2021, Adhemerval Zanella via Libc-alpha wrote:

> +static void
> +do_cleanup (void)
> +{
> +  xchdir ("..");
> +  for (int i = 0; i < NSUBFOLDERS - 1; i++)
> +    {
> +      remove (folder);
> +      xchdir ("..");
> +    }
> +  remove (folder);
> +}

This may go too far if the setup in do_test was incomplete.

Andreas.
diff mbox series

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 ce1c6a14a3..bb3ac0a7e0 100644
--- a/io/ftw.c
+++ b/io/ftw.c
@@ -392,13 +392,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..ecebe1e7c0
--- /dev/null
+++ b/io/tst-ftw-bz28126.c
@@ -0,0 +1,79 @@ 
+/* 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 <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 void
+do_cleanup (void)
+{
+  xchdir ("..");
+  for (int i = 0; i < NSUBFOLDERS - 1; i++)
+    {
+      remove (folder);
+      xchdir ("..");
+    }
+  remove (folder);
+}
+#define CLEANUP_HANDLER do_cleanup
+
+/* Check whether stack overflow occurs.  */
+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++)
+    {
+      xmkdir (folder, 0777);
+      xchdir (folder);
+    }
+
+  TEST_COMPARE (ftw (tempdir, my_func, 20), 0);
+
+  free (tempdir);
+
+  do_cleanup ();
+
+  return 0;
+}
+
+#include <support/test-driver.c>