thr3ads.net - Libguestfs - [Libguestfs] minor Hivex.xs leaks [Jun 2011]

If this information is useful, please help other people find it:
Share via:

Jim Meyering

2011-Jun-28 19:03 UTC

[Libguestfs] minor Hivex.xs leaks

Hi Rich,

While I was looking at hivex today I ran coverity on it.
It spotted one problem but missed a similar one nearby.

The following are from Hivex.xs: (generated by generator.ml)

void
node_set_values (h, node, values)
      hive_h *h;
      int node;
      pl_set_values values = unpack_pl_set_values (ST(2));
PREINIT:
      int r;
 PPCODE:
      r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
      free (values.values);
      if (r == -1)
        croak ("%s: %s", "node_set_values", strerror
(errno));

void
node_set_value (h, node, val)
      hive_h *h;
      int node;
      hive_set_value *val = unpack_set_value (ST(2));
PREINIT:
      int r;
 PPCODE:
      r = hivex_node_set_value (h, node, val, 0);
      free (val);
      if (r == -1)
        croak ("%s: %s", "node_set_value", strerror
(errno));

--------------------------------------------
Here's the generated C.
Note how each function uses XSRETURN_UNDEF
without freeing the "values" or "val" data they
have just allocated:

XS(XS_Win__Hivex_node_set_values); /* prototype to pass -Wmissing-prototypes */
XS(XS_Win__Hivex_node_set_values)
{
#ifdef dVAR
    dVAR; dXSARGS;
#else
    dXSARGS;
#endif
    if (items != 3)
       croak_xs_usage(cv,  "h, node, values");
    PERL_UNUSED_VAR(ax); /* -Wall */
    SP -= items;
    {
	hive_h *	h;
	int	node = (int)SvIV(ST(1));
	pl_set_values	values = unpack_pl_set_values (ST(2));
#line 477 "Hivex.xs"
      int r;
#line 993 "Hivex.c"

    if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
        h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
    else {
        warn ("Win::Hivex::node_set_values(): h is not a blessed SV
reference");
        XSRETURN_UNDEF;
    };
#line 479 "Hivex.xs"
      r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
      free (values.values);
      if (r == -1)
        croak ("%s: %s", "node_set_values", strerror
(errno));
#line 1006 "Hivex.c"
	PUTBACK;
	return;
    }
}


XS(XS_Win__Hivex_node_set_value); /* prototype to pass -Wmissing-prototypes */
XS(XS_Win__Hivex_node_set_value)
{
#ifdef dVAR
    dVAR; dXSARGS;
#else
    dXSARGS;
#endif
    if (items != 3)
       croak_xs_usage(cv,  "h, node, val");
    PERL_UNUSED_VAR(ax); /* -Wall */
    SP -= items;
    {
	hive_h *	h;
	int	node = (int)SvIV(ST(1));
	hive_set_value *	val = unpack_set_value (ST(2));
#line 490 "Hivex.xs"
      int r;
#line 1031 "Hivex.c"

    if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
        h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
    else {
        warn ("Win::Hivex::node_set_value(): h is not a blessed SV
reference");
        XSRETURN_UNDEF;
    };
#line 492 "Hivex.xs"
      r = hivex_node_set_value (h, node, val, 0);
      free (val);
      if (r == -1)
        croak ("%s: %s", "node_set_value", strerror
(errno));
#line 1044 "Hivex.c"
	PUTBACK;
	return;
    }
}
--------------------------------------------

One way to fix it is to change generator.ml to induce this
change in Hivex.xs:

--- Hivex.xs.~1~	2011-06-28 21:01:28.374623171 +0200
+++ Hivex.xs	2011-06-28 21:01:43.351623367 +0200
@@ -472,10 +472,10 @@ void
 node_set_values (h, node, values)
       hive_h *h;
       int node;
-      pl_set_values values = unpack_pl_set_values (ST(2));
 PREINIT:
       int r;
  PPCODE:
+      pl_set_values values = unpack_pl_set_values (ST(2));
       r = hivex_node_set_values (h, node, values.nr_values, values.values, 0);
       free (values.values);
       if (r == -1)
@@ -485,10 +485,10 @@ void
 node_set_value (h, node, val)
       hive_h *h;
       int node;
-      hive_set_value *val = unpack_set_value (ST(2));
 PREINIT:
       int r;
  PPCODE:
+      hive_set_value *val = unpack_set_value (ST(2));
       r = hivex_node_set_value (h, node, val, 0);
       free (val);
       if (r == -1)

Richard W.M. Jones

2011-Jun-28 22:38 UTC

head link

[Libguestfs] minor Hivex.xs leaks

On Tue, Jun 28, 2011 at 09:03:52PM +0200, Jim Meyering
wrote:> Hi Rich,
> 
> While I was looking at hivex today I ran coverity on it.
> It spotted one problem but missed a similar one nearby.
> 
> The following are from Hivex.xs: (generated by generator.ml)
> 
> void
> node_set_values (h, node, values)
>       hive_h *h;
>       int node;
>       pl_set_values values = unpack_pl_set_values (ST(2));
> PREINIT:
>       int r;
>  PPCODE:
>       r = hivex_node_set_values (h, node, values.nr_values, values.values,
0);
>       free (values.values);
>       if (r == -1)
>         croak ("%s: %s", "node_set_values", strerror
(errno));
> 
> void
> node_set_value (h, node, val)
>       hive_h *h;
>       int node;
>       hive_set_value *val = unpack_set_value (ST(2));
> PREINIT:
>       int r;
>  PPCODE:
>       r = hivex_node_set_value (h, node, val, 0);
>       free (val);
>       if (r == -1)
>         croak ("%s: %s", "node_set_value", strerror
(errno));
> 
> --------------------------------------------
> Here's the generated C.
> Note how each function uses XSRETURN_UNDEF
> without freeing the "values" or "val" data they
> have just allocated:
> 
> XS(XS_Win__Hivex_node_set_values); /* prototype to pass
-Wmissing-prototypes */
> XS(XS_Win__Hivex_node_set_values)
> {
> #ifdef dVAR
>     dVAR; dXSARGS;
> #else
>     dXSARGS;
> #endif
>     if (items != 3)
>        croak_xs_usage(cv,  "h, node, values");
>     PERL_UNUSED_VAR(ax); /* -Wall */
>     SP -= items;
>     {
> 	hive_h *	h;
> 	int	node = (int)SvIV(ST(1));
> 	pl_set_values	values = unpack_pl_set_values (ST(2));
> #line 477 "Hivex.xs"
>       int r;
> #line 993 "Hivex.c"
> 
>     if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
>         h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
>     else {
>         warn ("Win::Hivex::node_set_values(): h is not a blessed SV
reference");
>         XSRETURN_UNDEF;
>     };
> #line 479 "Hivex.xs"
>       r = hivex_node_set_values (h, node, values.nr_values, values.values,
0);
>       free (values.values);
>       if (r == -1)
>         croak ("%s: %s", "node_set_values", strerror
(errno));
> #line 1006 "Hivex.c"
> 	PUTBACK;
> 	return;
>     }
> }
> 
> 
> XS(XS_Win__Hivex_node_set_value); /* prototype to pass -Wmissing-prototypes
*/
> XS(XS_Win__Hivex_node_set_value)
> {
> #ifdef dVAR
>     dVAR; dXSARGS;
> #else
>     dXSARGS;
> #endif
>     if (items != 3)
>        croak_xs_usage(cv,  "h, node, val");
>     PERL_UNUSED_VAR(ax); /* -Wall */
>     SP -= items;
>     {
> 	hive_h *	h;
> 	int	node = (int)SvIV(ST(1));
> 	hive_set_value *	val = unpack_set_value (ST(2));
> #line 490 "Hivex.xs"
>       int r;
> #line 1031 "Hivex.c"
> 
>     if (sv_isobject (ST(0)) && SvTYPE (SvRV (ST(0))) == SVt_PVMG)
>         h = (hive_h *) SvIV ((SV *) SvRV (ST(0)));
>     else {
>         warn ("Win::Hivex::node_set_value(): h is not a blessed SV
reference");
>         XSRETURN_UNDEF;
>     };
> #line 492 "Hivex.xs"
>       r = hivex_node_set_value (h, node, val, 0);
>       free (val);
>       if (r == -1)
>         croak ("%s: %s", "node_set_value", strerror
(errno));
> #line 1044 "Hivex.c"
> 	PUTBACK;
> 	return;
>     }
> }
> --------------------------------------------
> 
> One way to fix it is to change generator.ml to induce this
> change in Hivex.xs:
> 
> --- Hivex.xs.~1~	2011-06-28 21:01:28.374623171 +0200
> +++ Hivex.xs	2011-06-28 21:01:43.351623367 +0200
> @@ -472,10 +472,10 @@ void
>  node_set_values (h, node, values)
>        hive_h *h;
>        int node;
> -      pl_set_values values = unpack_pl_set_values (ST(2));
>  PREINIT:
>        int r;
>   PPCODE:
> +      pl_set_values values = unpack_pl_set_values (ST(2));
>        r = hivex_node_set_values (h, node, values.nr_values, values.values,
0);
>        free (values.values);
>        if (r == -1)
> @@ -485,10 +485,10 @@ void
>  node_set_value (h, node, val)
>        hive_h *h;
>        int node;
> -      hive_set_value *val = unpack_set_value (ST(2));
>  PREINIT:
>        int r;
>   PPCODE:
> +      hive_set_value *val = unpack_set_value (ST(2));
>        r = hivex_node_set_value (h, node, val, 0);
>        free (val);
>        if (r == -1)
Tricky change to the generator.  I'll have to think about this a bit
more tomorrow ...

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

Richard W.M. Jones

2011-Jul-07 13:20 UTC

head link

[Libguestfs] minor Hivex.xs leaks

I don't think I'm going to get around to this soon, so I have
filed a bug to track this issue:

https://bugzilla.redhat.com/show_bug.cgi?id=719613

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

Possibly Parallel Threads

Search for more possibly parallel threads

Libguestfs - Jun 2011 - minor Hivex.xs leaks

[Libguestfs] minor Hivex.xs leaks

[Libguestfs] minor Hivex.xs leaks

[Libguestfs] minor Hivex.xs leaks

Possibly Parallel Threads