diff mbox series

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

Message ID 20210825142301.1763760-1-adhemerval.zanella@linaro.org
State Superseded
Headers
Series 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. 25, 2021, 2:23 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.
---
 io/Makefile          |  1 +
 io/ftw.c             | 10 +++---
 io/tst-ftw-bz28126.c | 79 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 4 deletions(-)
 create mode 100644 io/tst-ftw-bz28126.c

Comments

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

> diff --git a/io/ftw.c b/io/ftw.c
> index ce1c6a14a3..069af37f43 100644
> --- a/io/ftw.c
> +++ b/io/ftw.c
> @@ -392,13 +392,15 @@ 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);
> +      void *newp = realloc (data->dirstreams, data->maxdir
> +					      * sizeof (struct dir_data *)
> +					      + data->dirbufsize);

I think data->dirbufsize should only be modified if realloc was
successful.

Andreas.
Adhemerval Zanella Aug. 25, 2021, 3:06 p.m. UTC | #2 
On 25/08/2021 11:50, Andreas Schwab wrote:
> On Aug 25 2021, Adhemerval Zanella via Libc-alpha wrote:
> 
>> diff --git a/io/ftw.c b/io/ftw.c
>> index ce1c6a14a3..069af37f43 100644
>> --- a/io/ftw.c
>> +++ b/io/ftw.c
>> @@ -392,13 +392,15 @@ 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);
>> +      void *newp = realloc (data->dirstreams, data->maxdir
>> +					      * sizeof (struct dir_data *)
>> +					      + data->dirbufsize);
> 
> I think data->dirbufsize should only be modified if realloc was
> successful.

Indeed, I will update it.
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..069af37f43 100644
--- a/io/ftw.c
+++ b/io/ftw.c
@@ -392,13 +392,15 @@  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);
+      void *newp = realloc (data->dirstreams, data->maxdir
+					      * sizeof (struct dir_data *)
+					      + data->dirbufsize);
       if (newp == NULL)
 	return -1;
-      data->dirbuf = newp;
+      data->dirstreams = newp;
+      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>