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)
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
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
Seemingly Similar Threads
- [PATCH] hivex: add hivex_set_value api call and ocaml/perl bindings, tests
- [PATCH] hivex: add hivex_set_value api call and perl bindings, tests
- [hivex] Segfault for an integer value to node_set_value
- [hivex] [PATCH 0/6] Python fixes for node_set_value
- [PATCH] hivex: add hivex_set_value api call