Re: [Libguestfs] [PATCH] daemon: readdir: fix invalid memory access on error

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().