Re: [Xen-devel] [PATCH] libxenlight: fix heap overflow when domid_to_name returns NULL

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

On Mon, 2011-03-07 at 15:04 +0000, Stefano Stabellini wrote:
> On Mon, 7 Mar 2011, Ian Campbell wrote:
> > On Mon, 2011-03-07 at 13:12 +0000, Stefano Stabellini wrote:
> > > On Fri, 4 Mar 2011, Ian Campbell wrote:
> > > > On Thu, 2011-03-03 at 23:29 +0000, Eamon Walsh wrote:
> > > > > The function flexarray_vappend() will stop at the first NULL
> > > > > argument.  In libxl_device_vfb_add(), this has been observed
> > > > > to result in keys being added to the backend array without
> > > > > associated values in cases where the value can be NULL.
> > > > 
> > > > If these values are NULL should we be writing them at all? e.g. for:
> > > >         flexarray_vappend(back, foo, bar);
> > > > where bar may be NULL shouldn't it become:
> > > >         if (bar) 
> > > >                 flexarray_vappend(back, foo, bar);
> > > > or perhaps:
> > > >         flexarray_vappend(back, foo, bar ? bar : "");
> > > > ?
> > > > 
> > > 
> > > This is actually a serious issue because it means that every time
> > > flexarray_vappend is used and the argument is NULL the behaviour is
> > > going to be different from what the coder expected.
> > > Maybe flexarray_vappend should assume that the number of args is odd and
> > > greater than 2?
> > 
> > flexarray_vappend is not solely used with libxl__xs_kvs_of_flexarray
> > though, in other cases it may be perfectly valid to have an odd number
> > of items.
> > 
> > flexarray_vappend_pair() perhaps?
> 
> flexarray_vappend_pairs considering that it is going to take a variable
> number of pairs?

That was what I was thinking at the time...

> Otherwise we might as well have a flexarray_append_pair that doesn't use
> vargs at all.

... but I think this makes more sense. Anywhere which actually does use
vappend to append multiple pairs would probably be clearer as a series
of these anyway.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel