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

[Hu Tao]

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

Regards,
Hu

> +      } else {
> +        free_int_dirent_list (ret->guestfs_int_dirent_list_val, i);
> +      }
>        free (v.name);
>        free (ret);
>        closedir (dir);
> -- 
> 1.9.3
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs at redhat.com
> https://www.redhat.com/mailman/listinfo/libguestfs