From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v5 10/12] libxl: ocaml: fix memory corruption when converting string and key/values lists Date: Tue, 26 Nov 2013 18:01:51 +0000 Message-ID: <5294E20F.7050500@citrix.com> References: <1385488371-28875-1-git-send-email-rob.hoes@citrix.com> <1385488371-28875-11-git-send-email-rob.hoes@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1385488371-28875-11-git-send-email-rob.hoes@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Rob Hoes Cc: ian.jackson@citrix.com, dave.scott@eu.citrix.com, ian.campbell@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 26/11/13 17:52, Rob Hoes wrote: > Found by Coverty. CIDs: 1128562 1128563 1128564 1128565. > > Signed-off-by: Rob Hoes It is worth further stating that this is due to incorrect indirections, just like b0be2b126ea75a83a3778b4e1710d248f92cf528 Reviewed-by: Andrew Cooper FWIW, the libxl_string_list tyepdef makes it far too easy to do this. It might be worth trying to turn it into an opaque type to reduce these kinds of errors. > --- > tools/ocaml/libs/xl/xenlight_stubs.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/tools/ocaml/libs/xl/xenlight_stubs.c b/tools/ocaml/libs/xl/xenlight_stubs.c > index 7012045..a2d47f9 100644 > --- a/tools/ocaml/libs/xl/xenlight_stubs.c > +++ b/tools/ocaml/libs/xl/xenlight_stubs.c > @@ -159,8 +159,8 @@ static value Val_key_value_list(libxl_key_value_list *c_val) > > list = Val_emptylist; > for (i = libxl_string_list_length((libxl_string_list *) c_val) - 1; i >= 0; i -= 2) { > - val = caml_copy_string((char *) c_val[i]); > - key = caml_copy_string((char *) c_val[i - 1]); > + val = caml_copy_string((*c_val)[i]); > + key = caml_copy_string((*c_val)[i - 1]); > kv = caml_alloc_tuple(2); > Store_field(kv, 0, key); > Store_field(kv, 1, val); > @@ -201,7 +201,7 @@ static value Val_string_list(libxl_string_list *c_val) > > list = Val_emptylist; > for (i = libxl_string_list_length(c_val) - 1; i >= 0; i--) { > - string = caml_copy_string((char *) c_val[i]); > + string = caml_copy_string((*c_val)[i]); > cons = caml_alloc(2, 0); > Store_field(cons, 0, string); // head > Store_field(cons, 1, list); // tail