On 12/05/18 18:50, ISE Development wrote:
> Hi,
>
> I may be wrong but I suspect there is a corner case where fts_close()
> will not free the FTSENT structures correctly if called immediately
> after fts_open().
>
> After fts_open(), the current entry is a dummy entry created as
> follows:
>
> if ((sp->fts_cur = fts_alloc(sp, "", 0)) == NULL)
> goto mem3;
> sp->fts_cur->fts_link = root;
> sp->fts_cur->fts_info = FTS_INIT;
>
> It would normally be freed during the first invocation of fts_read().
>
> In fts_close():
>
> if (sp->fts_cur) {
> for (p = sp->fts_cur; p->fts_level >= FTS_ROOTLEVEL;) {
> freep = p;
> p = p->fts_link != NULL ? p->fts_link : p->fts_parent;
> free(freep);
> }
> free(p);
> }
>
> However, fts_alloc() does not clear or set fts_level, nor does it zero
> the entire FTSENT structure.
>
> So as far as I can figure, it is possible for the fts_level of the
> dummy entry to be negative after fts_open() causing fts_close() not to
> free the actual root level entries.
Yes valgrind indicates that fts_level is uninitialized if you
fts_close() right after fts_open().
The attached should fix it up.
thanks!
Pádraig
From 71b6724aa2e2843da7b73151d13c678452a59c7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sun, 13 May 2018 18:42:37 -0700
Subject: [PATCH] fts: avoid a memory leak edge case
* lib/fts.c (fts_open): Set an appropriate fts_level
so that an immediate fts_close() will free the allocation.
* tests/test-fts.c (fts_dealloc): Add a test case which
will trigger under valgrind or address sanitizer.
Fixes https://bugs.gnu.org/31439
---
ChangeLog | 9 +++++++++
lib/fts.c | 1 +
tests/test-fts.c | 23 ++++++++++++++++++++++-
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 24fd4da..e1b9c7e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,12 @@
+2018-05-13 Pádraig Brady <p...@draigbrady.com>
+
+ fts: avoid a memory leak edge case
+ * lib/fts.c (fts_open): Set an appropriate fts_level
+ so that an immediate fts_close() will free the allocation.
+ * tests/test-fts.c (fts_dealloc): Add a test case which
+ will trigger under valgrind or address sanitizer.
+ Fixes https://bugs.gnu.org/31439
+
2018-05-13 Bruno Haible <br...@clisp.org>
nl_langinfo: Fix compilation error on Android.
diff --git a/lib/fts.c b/lib/fts.c
index d543510..1ccc78c 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -546,6 +546,7 @@ fts_open (char * const *argv,
goto mem3;
sp->fts_cur->fts_link = root;
sp->fts_cur->fts_info = FTS_INIT;
+ sp->fts_cur->fts_level = 1;
if (! setup_dir (sp))
goto mem3;
diff --git a/tests/test-fts.c b/tests/test-fts.c
index ad15aff..a9c1dd8 100644
--- a/tests/test-fts.c
+++ b/tests/test-fts.c
@@ -38,6 +38,23 @@ perror_exit (char const *message, int status)
exit (status);
}
+/* alloc/dealloc to ensure structures initialized appropriately. */
+static void
+fts_dealloc (void)
+{
+ static char dir[] = "./";
+ static char *const curr_dir[2] = { dir, 0 };
+ FTSENT *e;
+ FTS *ftsp = fts_open (curr_dir, FTS_NOSTAT | FTS_PHYSICAL | FTS_CWDFD, 0);
+ if (ftsp)
+ {
+ if (fts_close (ftsp) != 0)
+ perror_exit ("fts_close", 9);
+ }
+ else
+ perror_exit (base, 10);
+}
+
/* Remove BASE and all files under it. */
static void
remove_tree (void)
@@ -122,9 +139,10 @@ main (void)
perror_exit (base, 6);
while ((e = fts_read (ftsp)))
needles_seen += strcmp (e->fts_name, "needle") == 0;
- fflush (stdout);
if (errno)
perror_exit ("fts_read", 7);
+ if (fts_close (ftsp) != 0)
+ perror_exit (base, 8);
/* Report an error if we did not find the needles. */
if (needles_seen != needles)
@@ -140,5 +158,8 @@ main (void)
fprintf (stderr, "fts could not remove directory\n");
return 1;
}
+
+ fts_dealloc ();
+
return 0;
}
--
2.9.3
