Libguestfs - Nov 2014 - [libhivex] Memory leak in hivex_node_delete_child?

[This email is either empty or too large to be displayed at this time]

Hi,

On Monday 10 November 2014 18:28:52 Mahmoud Al-Qudsi
wrote:
> I’m not able to pin it myself from a brief look, but it seems that
> under certain conditions a call to hivex_node_delete_child can cause
> the allocations from _hivex_get_children to not be freed properly?
> 
> I know that if the return value of _hivex_get_children is -1, no free
> is called; but from what I can see, -1 means there is nothing to be
> freed/it’s been taken care of.
> 
> I’ve attached the valgrind log together with the output of
> HIVEX_OPEN_DEBUG, please let me know if there’s anything else I can
> provide to make this easier.
It seems like rather hivex_node_delete_child leaks the "blocks" array 
returned by _hivex_get_children every time. The patch should be easy, 
would you be able to give it a try to check whether it fixes the leak in 
your application?

Thanks,
-- 
Pino Toscano

> On Nov 11, 2014, at 4:41 AM, Pino Toscano <ptoscano@redhat.com>
wrote:
> 
> Hi,
> 
> It seems like rather hivex_node_delete_child leaks the "blocks"
array
> returned by _hivex_get_children every time. The patch should be easy, 
> would you be able to give it a try to check whether it fixes the leak in 
> your application?
Awesome, thanks!

It does indeed look like the right (and easy) fix. I was concentrating my effort
on the exceptions as it didn't occur to me it was in the normal course of
action!

Thanks. I'll test it and be back. 

Cheers,

Mahmoud

> On Nov 11, 2014, at 5:36 AM, Mahmoud Al-Qudsi <mqudsi@neosmart.net>
wrote:
> I'll test it and be back. 
And it works perfectly. 

Thanks,

Mahmoud

On Tue, Nov 11, 2014 at 11:41:33AM +0100, Pino Toscano
wrote:
> Hi,
> 
> On Monday 10 November 2014 18:28:52 Mahmoud Al-Qudsi wrote:
> > I’m not able to pin it myself from a brief look, but it seems that
> > under certain conditions a call to hivex_node_delete_child can cause
> > the allocations from _hivex_get_children to not be freed properly?
> > 
> > I know that if the return value of _hivex_get_children is -1, no free
> > is called; but from what I can see, -1 means there is nothing to be
> > freed/it’s been taken care of.
> > 
> > I’ve attached the valgrind log together with the output of
> > HIVEX_OPEN_DEBUG, please let me know if there’s anything else I can
> > provide to make this easier.
> 
> It seems like rather hivex_node_delete_child leaks the "blocks"
array
> returned by _hivex_get_children every time. The patch should be easy, 
> would you be able to give it a try to check whether it fixes the leak in 
> your application?
> 
> Thanks,
> -- 
> Pino Toscano
> >From 854a8e763a83870e8248e132ec3a9a063f334035 Mon Sep 17 00:00:00 2001
> From: Pino Toscano <ptoscano@redhat.com>
> Date: Tue, 11 Nov 2014 11:36:30 +0100
> Subject: [PATCH] lib: write: fix memory leak
> 
> Free the "blocks" array got from _hivex_get_children.
> 
> Thanks: Mahmoud Al-Qudsi
> ---
>  lib/write.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/write.c b/lib/write.c
> index abd12c5..33b64e4 100644
> --- a/lib/write.c
> +++ b/lib/write.c
> @@ -902,10 +902,13 @@ hivex_node_delete_child (hive_h *h, hive_node_h node)
>          }
>      }
>    }
> +  free (blocks);
>    SET_ERRNO (ENOTSUP, "could not find parent to child link");
>    return -1;
>  
>   found:;
> +  free (blocks);
> +
>    struct ntreg_nk_record *nk >      (struct ntreg_nk_record *) ((char
*) h->addr + parent);
>    size_t nr_subkeys_in_nk = le32toh (nk->nr_subkeys);
> -- 
> 1.9.3
ACK.  I've included this in my tree, and I'm testing it using afl-fuzz.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW