From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eamon Walsh Subject: Re: [PATCH] libxenlight: fix heap overflow when domid_to_name returns NULL Date: Mon, 07 Mar 2011 10:33:03 -0500 Message-ID: <4D74FAAF.7040105@tycho.nsa.gov> References: <4D702468.9040206@tycho.nsa.gov> <1299232954.6552.242.camel@zakaz.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1299232954.6552.242.camel@zakaz.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Ian Campbell Cc: Xen-devel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org On 03/04/2011 05:02 AM, 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 : ""); > ? > If the value is NULL, the key is skipped and not written. This is because of a patch I submitted to change the xs_writev() function, which was calling strlen(NULL) previously. See: http://lists.xensource.com/archives/html/xen-devel/2010-03/msg00703.html However this behavior is not obvious. Checking the value earlier and leaving it off the list makes sense. -- Eamon Walsh National Security Agency