On Wednesday 21 January 2015 10:28:33 Hu Tao wrote:
On Tue, Jan 20, 2015 at 04:28:39PM +0100, Pino Toscano wrote:
If "strdup (d->d_name)" fails with "i" > 0, then both "p" and "ret->guestfs_int_dirent_list_val" are non-null pointers, but the latter is no more valid (since "p" is the new realloc'ed buffer). Hence, trying to free both will access to invalid memory.
Make sure to free only one of them, "p" if not null or "ret->guestfs_int_dirent_list_val" otherwise. --- daemon/readdir.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/daemon/readdir.c b/daemon/readdir.c index f0ddd21..e488f93 100644 --- a/daemon/readdir.c +++ b/daemon/readdir.c @@ -27,6 +27,17 @@ #include "daemon.h" #include "actions.h"
+static void +free_int_dirent_list (guestfs_int_dirent *p, size_t len) +{ + size_t i; + + for (i = 0; i < len; ++i) { + free (p[i].name); + } + free (p); +} + guestfs_int_dirent_list * do_readdir (const char *path) { @@ -64,8 +75,11 @@ do_readdir (const char *path) v.name = strdup (d->d_name); if (!p || !v.name) { reply_with_perror ("allocate"); - free (ret->guestfs_int_dirent_list_val); - free (p); + if (p) { + free_int_dirent_list (p, i);
I think free() is the correct way to free memory here, since the memory is allocated by realloc().
free_int_dirent_list in the end calls free on the whole array, but each element has a char* member which need to be freed too. -- Pino Toscano