From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Roger_Pau_Monn=E9?= Subject: Re: xl network-attach SEGV in 4.2 and 4.1 Date: Thu, 18 Apr 2013 20:58:20 +0200 Message-ID: <5170424C.4080103@citrix.com> References: <516E0DCF.9090001@invisiblethingslab.com> <1366188196.8399.187.camel@zakaz.uk.xensource.com> <20848.6355.859738.100856@mariner.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20848.6355.859738.100856@mariner.uk.xensource.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: Ian Jackson Cc: "xen-devel@lists.xen.org" , Ian Campbell , Marek Marczykowski List-Id: xen-devel@lists.xenproject.org On 18/04/13 18:01, Ian Jackson wrote: > Ian Campbell writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and 4= .1"): >> On Wed, 2013-04-17 at 03:49 +0100, Marek Marczykowski wrote: >>> Hi all, >>> When "device/vif" directory exists but is empty l!=3DNULL, but nb=3D=3D= 0, so >>> l[nb-1] is invalid. Add missing check. > = > Thanks for pointing this out. The root cause is the expectation that > libxl__xs_directory won't return a non-null list but set *nb to 0. > = > I found some occurrences of this bug in (xen-unstable) staging. > See patch below, which should probably go into 4.1 and 4.2. > = >>> Signed-Off-by: Marek Marczykowski >>> >>> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c >>> index 5783cd2..9e06a7d 100644 >>> --- a/tools/libxl/libxl.c >>> +++ b/tools/libxl/libxl.c >>> @@ -2569,7 +2569,8 @@ void libxl__device_nic_add(libxl__egc *egc, uint3= 2_t >>> domid, >>> goto out_free; >>> } >>> if (!(l =3D libxl__xs_directory(gc, XBT_NULL, >>> - libxl__sprintf(gc, "%s/device/vif= ", >>> dompath), &nb))) { >>> + libxl__sprintf(gc, "%s/device/vif= ", >>> dompath), &nb)) || >>> + nb =3D=3D 0) { > = > I think this is the right way to fix this in the stable trees. I > looked at the backport of "5420f2650 libxl: Set vfb and vkb devid if > not done so by the caller" and it's nontrivial, so although it's a > bugfix I'd rather leave it. > = > Ian. > = > = > commit d1f52920ee96c91665813fb875c0c76e5eb7a312 > Author: Ian Jackson > Date: Thu Apr 18 16:27:46 2013 +0100 > = > libxl: Avoid realloc(,0) when libxl__xs_directory returns empty list > = > If the named path is a leaf node, libxl__xs_directory can succeed, > returning non-null, but set *nb to 0. > = > In three places in libxl this may result in a zero size argument being > passed to malloc() or realloc(), which is not adviseable. > = > Signed-off-by: Ian Jackson Acked-by: Roger Pau Monn=E9 > = > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index 0f936c0..1c04d37 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -1840,7 +1840,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx= *ctx, uint32_t domid, int *n > = > fe_path =3D libxl__sprintf(gc, "%s/device/vtpm", libxl__xs_get_dompa= th(gc, domid)); > dir =3D libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs); > - if(dir) { > + if (dir && ndirs) { > vtpms =3D malloc(sizeof(*vtpms) * ndirs); > libxl_device_vtpm* vtpm; > libxl_device_vtpm* end =3D vtpms + ndirs; > @@ -2314,7 +2314,7 @@ static int libxl__append_disk_list_of_type(libxl__g= c *gc, > be_path =3D libxl__sprintf(gc, "%s/backend/%s/%d", > libxl__xs_get_dompath(gc, 0), type, domid); > dir =3D libxl__xs_directory(gc, XBT_NULL, be_path, &n); > - if (dir) { > + if (dir && n) { > libxl_device_disk *tmp; > tmp =3D realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + = n)); > if (tmp =3D=3D NULL) > @@ -3006,7 +3006,7 @@ static int libxl__append_nic_list_of_type(libxl__gc= *gc, > be_path =3D libxl__sprintf(gc, "%s/backend/%s/%d", > libxl__xs_get_dompath(gc, 0), type, domid); > dir =3D libxl__xs_directory(gc, XBT_NULL, be_path, &n); > - if (dir) { > + if (dir && n) { > libxl_device_nic *tmp; > tmp =3D realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n)); > if (tmp =3D=3D NULL) > = > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > =