thr3ads.net - Xen devel - xl network-attach SEGV in 4.2 and 4.1 [Apr 2013]

If this information is useful, please help other people find it:
Share via:

Marek Marczykowski

2013-Apr-17 02:49 UTC

xl network-attach SEGV in 4.2 and 4.1

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi all,

4.2 and 4.1 suffers from SEGV during xl network-attach in
libxl__device_nic_add. In 4.3-unstable it is fixed by:
5420f2650 libxl: Set vfb and vkb devid if not done so by the caller

So either the patch need to be backported to 4.1 and 4.2, or fixed by this one:
- ------
libxl: Fix SEGV in network-attach

When "device/vif" directory exists but is empty l!=NULL, but nb==0, so
l[nb-1] is invalid. Add missing check.

Signed-Off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>

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, uint32_t
domid,
             goto out_free;
         }
         if (!(l = libxl__xs_directory(gc, XBT_NULL,
- -                                     libxl__sprintf(gc,
"%s/device/vif",
dompath), &nb))) {
+                                     libxl__sprintf(gc,
"%s/device/vif",
dompath), &nb)) ||
+                nb == 0) {
             nic->devid = 0;
         } else {
             nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;

- -- 
Best Regards / Pozdrawiam,
Marek Marczykowski
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (GNU/Linux)
Comment: Using GnuPG with undefined - http://www.enigmail.net/

iQEcBAEBAgAGBQJRbg3PAAoJENuP0xzK19cs0qQH/2mR6lUwj75tKCckFJfM7HPJ
6YEqDvdunmCXr7Wcf9p+Z6RRsnjie9ihDwxbaP4b5XYMLW59YbvZEb3tyrIADYJy
gGwE+8xn3Jx0NjiRCJF5O2nF3w9I5W9o5sUMbXporo/Tu4X0e3PJ6wb1JseZWsLL
mVl3bjwIChos6eKsAKOuHIPqFCASsKIGJibrkWva9LqQ4k/xgjTshCgmbAgEITEo
8fAZbTRL/SM91qX5a+2ynUf/0h5Sb/IvLyLwm86n1wsqLoJ261VMgAOsXxYGUTBM
Pr5Gvc+s5VdhA0iY4IYE8GuQ99MnaXO6T+JSenpfJ3L26LSMvHkImFLqI3ljhzE=FWNX
-----END PGP SIGNATURE-----

Ian Campbell

2013-Apr-17 08:43 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1

Copying the stable tools maintainer. Please always do so for backport
requests (see MAINTAINERS in the relevant tree to see who to copy).

On Wed, 2013-04-17 at 03:49 +0100, Marek Marczykowski
wrote:> Hi all,
> 
> 4.2 and 4.1 suffers from SEGV during xl network-attach in
> libxl__device_nic_add. In 4.3-unstable it is fixed by:
> 5420f2650 libxl: Set vfb and vkb devid if not done so by the caller
> 
> So either the patch need to be backported to 4.1 and 4.2,
On the face of it I can''t see any problem with backporting that commit
>  or fixed by this one:
> ------
> libxl: Fix SEGV in network-attach
> 
> When "device/vif" directory exists but is empty l!=NULL, but
nb==0, so
> l[nb-1] is invalid. Add missing check.
> 
> Signed-Off-by: Marek Marczykowski <marmarek@invisiblethingslab.com>
> 
> 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, uint32_t
> domid,
>              goto out_free;
>          }
>          if (!(l = libxl__xs_directory(gc, XBT_NULL,
> -                                     libxl__sprintf(gc,
"%s/device/vif",
> dompath), &nb))) {
> +                                     libxl__sprintf(gc,
"%s/device/vif",
> dompath), &nb)) ||
> +                nb == 0) {
>              nic->devid = 0;
>          } else {
>              nic->devid = strtoul(l[nb - 1], NULL, 10) + 1;
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Ian Jackson

2013-Apr-18 16:01 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1

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!=NULL, but
nb==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
<marmarek@invisiblethingslab.com>
> > 
> > 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,
uint32_t
> > domid,
> >              goto out_free;
> >          }
> >          if (!(l = libxl__xs_directory(gc, XBT_NULL,
> > -                                     libxl__sprintf(gc,
"%s/device/vif",
> > dompath), &nb))) {
> > +                                     libxl__sprintf(gc,
"%s/device/vif",
> > dompath), &nb)) ||
> > +                nb == 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 <ian.jackson@eu.citrix.com>
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 <Ian.Jackson@eu.citrix.com>

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 = libxl__sprintf(gc, "%s/device/vtpm",
libxl__xs_get_dompath(gc, domid));
     dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
-    if(dir) {
+    if (dir && ndirs) {
        vtpms = malloc(sizeof(*vtpms) * ndirs);
        libxl_device_vtpm* vtpm;
        libxl_device_vtpm* end = vtpms + ndirs;
@@ -2314,7 +2314,7 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc,
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
-    if (dir) {
+    if (dir && n) {
         libxl_device_disk *tmp;
         tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
         if (tmp == NULL)
@@ -3006,7 +3006,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
-    if (dir) {
+    if (dir && n) {
         libxl_device_nic *tmp;
         tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
         if (tmp == NULL)

Ian Jackson

2013-Apr-18 16:46 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

Marek Marczykowski writes ("[Xen-devel] xl network-attach SEGV in 4.2 and
4.1"):> 4.2 and 4.1 suffers from SEGV during xl network-attach in
> libxl__device_nic_add. In 4.3-unstable it is fixed by:
> 5420f2650 libxl: Set vfb and vkb devid if not done so by the caller
> 
> So either the patch need to be backported to 4.1 and 4.2, or fixed by this
one:
> - ------
> libxl: Fix SEGV in network-attach
I have applied this to 4.2 and 4.1.  I had to fix up a conflict in 4.1
and also I fixed a whitespace bug.  The patches as committed are below.
> When "device/vif" directory exists but is empty l!=NULL, but
nb==0, so
> l[nb-1] is invalid. Add missing check.
...>          if (!(l = libxl__xs_directory(gc, XBT_NULL,
> - -                                     libxl__sprintf(gc,
"%s/device/vif",
> dompath), &nb))) {
> +                                     libxl__sprintf(gc,
"%s/device/vif",
> dompath), &nb)) ||
> +                nb == 0) {
Something in the arrangements you used to prepare this patch
linewrapped it _before you signed it_.

Ian Jackson writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and
4.1"):> Author: Ian Jackson <ian.jackson@eu.citrix.com>
> Date:   Thu Apr 18 16:27:46 2013 +0100
> 
>     libxl: Avoid realloc(,0) when libxl__xs_directory returns empty list
IWBNI someone would review this and ack it; I''d like it to go into
4.1, 4.2 and unstable.

Ian.

Roger Pau Monné

2013-Apr-18 18:58 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1

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!=NULL,
but nb==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
<marmarek@invisiblethingslab.com>
>>>
>>> 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,
uint32_t
>>> domid,
>>>              goto out_free;
>>>          }
>>>          if (!(l = libxl__xs_directory(gc, XBT_NULL,
>>> -                                     libxl__sprintf(gc,
"%s/device/vif",
>>> dompath), &nb))) {
>>> +                                     libxl__sprintf(gc,
"%s/device/vif",
>>> dompath), &nb)) ||
>>> +                nb == 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 <ian.jackson@eu.citrix.com>
> 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 <Ian.Jackson@eu.citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> 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 = libxl__sprintf(gc, "%s/device/vtpm",
libxl__xs_get_dompath(gc, domid));
>      dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
> -    if(dir) {
> +    if (dir && ndirs) {
>         vtpms = malloc(sizeof(*vtpms) * ndirs);
>         libxl_device_vtpm* vtpm;
>         libxl_device_vtpm* end = vtpms + ndirs;
> @@ -2314,7 +2314,7 @@ static int libxl__append_disk_list_of_type(libxl__gc
*gc,
>      be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
>                               libxl__xs_get_dompath(gc, 0), type, domid);
>      dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
> -    if (dir) {
> +    if (dir && n) {
>          libxl_device_disk *tmp;
>          tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
>          if (tmp == NULL)
> @@ -3006,7 +3006,7 @@ static int libxl__append_nic_list_of_type(libxl__gc
*gc,
>      be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
>                               libxl__xs_get_dompath(gc, 0), type, domid);
>      dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
> -    if (dir) {
> +    if (dir && n) {
>          libxl_device_nic *tmp;
>          tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
>          if (tmp == NULL)
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>

Jan Beulich

2013-Apr-19 07:12 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

>>> On 18.04.13 at 18:46, Ian Jackson <Ian.Jackson@eu.citrix.com>
wrote:
> Marek Marczykowski writes ("[Xen-devel] xl network-attach SEGV in 4.2
and
> 4.1"):
>> 4.2 and 4.1 suffers from SEGV during xl network-attach in
>> libxl__device_nic_add. In 4.3-unstable it is fixed by:
>> 5420f2650 libxl: Set vfb and vkb devid if not done so by the caller
>> 
>> So either the patch need to be backported to 4.1 and 4.2, or fixed by
this
> one:
>> - ------
>> libxl: Fix SEGV in network-attach
> 
> I have applied this to 4.2 and 4.1.  I had to fix up a conflict in 4.1
> and also I fixed a whitespace bug.  The patches as committed are below.
This fixup on 4.1 went wrong - you dropped an & that''s necessary.
I''ll take the liberty to fix this right away.

Jan
>> When "device/vif" directory exists but is empty l!=NULL, but
nb==0, so
>> l[nb-1] is invalid. Add missing check.
> ...
>>          if (!(l = libxl__xs_directory(gc, XBT_NULL,
>> - -                                     libxl__sprintf(gc,
"%s/device/vif",
>> dompath), &nb))) {
>> +                                     libxl__sprintf(gc,
"%s/device/vif",
>> dompath), &nb)) ||
>> +                nb == 0) {
> 
> Something in the arrangements you used to prepare this patch
> linewrapped it _before you signed it_.
> 
> Ian Jackson writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and
4.1"):
>> Author: Ian Jackson <ian.jackson@eu.citrix.com>
>> Date:   Thu Apr 18 16:27:46 2013 +0100
>> 
>>     libxl: Avoid realloc(,0) when libxl__xs_directory returns empty
list
> 
> IWBNI someone would review this and ack it; I''d like it to go into
> 4.1, 4.2 and unstable.
> 
> Ian.

Ian Jackson

2013-Apr-22 09:49 UTC

head link

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

Jan Beulich writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and 4.1
[and 1 more messages]"):> This fixup on 4.1 went wrong - you dropped an & that''s
necessary.
> I''ll take the liberty to fix this right away.
Oops, thanks.

Ian.

Ian Jackson

2013-Nov-12 17:23 UTC

head link

[PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Ian Jackson writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and
4.1"):> 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.
Ping.

Ian.

From ea275c0e29299a2399aa7814116d68bfdaeb21c1 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@eu.citrix.com>
Date: Thu, 18 Apr 2013 16:27:46 +0100
Subject: [PATCH] libxl: Avoid realloc(,0) when libxl__xs_directory returns
 empty list
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 <Ian.Jackson@eu.citrix.com>
Acked-by: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/libxl/libxl.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 0f0f56c..887e0ca 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1864,7 +1864,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx *ctx,
uint32_t domid, int *n
 
     fe_path = libxl__sprintf(gc, "%s/device/vtpm",
libxl__xs_get_dompath(gc, domid));
     dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
-    if(dir) {
+    if (dir && ndirs) {
        vtpms = malloc(sizeof(*vtpms) * ndirs);
        libxl_device_vtpm* vtpm;
        libxl_device_vtpm* end = vtpms + ndirs;
@@ -2371,7 +2371,7 @@ static int libxl__append_disk_list_of_type(libxl__gc *gc,
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
-    if (dir) {
+    if (dir && n) {
         libxl_device_disk *tmp;
         tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
         if (tmp == NULL)
@@ -3060,7 +3060,7 @@ static int libxl__append_nic_list_of_type(libxl__gc *gc,
     be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
                              libxl__xs_get_dompath(gc, 0), type, domid);
     dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
-    if (dir) {
+    if (dir && n) {
         libxl_device_nic *tmp;
         tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
         if (tmp == NULL)
-- 
1.7.10.4

Ian Campbell

2013-Nov-12 17:31 UTC

head link

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

On Tue, 2013-11-12 at 17:23 +0000, Ian Jackson wrote:> Ian Jackson writes ("Re: [Xen-devel] xl network-attach SEGV in 4.2 and
4.1"):
> > 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.
> 
> Ping.
I could have sworn this went in...
> 
> Ian.
> 
> From ea275c0e29299a2399aa7814116d68bfdaeb21c1 Mon Sep 17 00:00:00 2001
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> Date: Thu, 18 Apr 2013 16:27:46 +0100
> Subject: [PATCH] libxl: Avoid realloc(,0) when libxl__xs_directory returns
>  empty list
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> 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.
I thought it was strictly speaking fine, but I can see why it would be
best to avoid.
> Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
> ---
>  tools/libxl/libxl.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 0f0f56c..887e0ca 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -1864,7 +1864,7 @@ libxl_device_vtpm *libxl_device_vtpm_list(libxl_ctx
*ctx, uint32_t domid, int *n
>  
>      fe_path = libxl__sprintf(gc, "%s/device/vtpm",
libxl__xs_get_dompath(gc, domid));
>      dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs);
> -    if(dir) {
> +    if (dir && ndirs) {
>         vtpms = malloc(sizeof(*vtpms) * ndirs);
>         libxl_device_vtpm* vtpm;
>         libxl_device_vtpm* end = vtpms + ndirs;
> @@ -2371,7 +2371,7 @@ static int libxl__append_disk_list_of_type(libxl__gc
*gc,
>      be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
>                               libxl__xs_get_dompath(gc, 0), type, domid);
>      dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
> -    if (dir) {
> +    if (dir && n) {
>          libxl_device_disk *tmp;
>          tmp = realloc(*disks, sizeof (libxl_device_disk) * (*ndisks + n));
>          if (tmp == NULL)
> @@ -3060,7 +3060,7 @@ static int libxl__append_nic_list_of_type(libxl__gc
*gc,
>      be_path = libxl__sprintf(gc, "%s/backend/%s/%d",
>                               libxl__xs_get_dompath(gc, 0), type, domid);
>      dir = libxl__xs_directory(gc, XBT_NULL, be_path, &n);
> -    if (dir) {
> +    if (dir && n) {
>          libxl_device_nic *tmp;
>          tmp = realloc(*nics, sizeof (libxl_device_nic) * (*nnics + n));
>          if (tmp == NULL)


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Ian Jackson

2013-Nov-12 17:57 UTC

head link

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Ian Campbell writes ("Re: [PATCH RESEND] libxl: Avoid realloc(,0) when
libxl__xs_directory returns empty list"):> On Tue, 2013-11-12 at 17:23 +0000, Ian Jackson wrote:
> > Ping.
> 
> I could have sworn this went in...
Yes.  I have found a bunch of these lying around in branches in my
main tree.
> > In three places in libxl this may result in a zero size argument being
> > passed to malloc() or realloc(), which is not adviseable.
> 
> I thought it was strictly speaking fine, but I can see why it would be
> best to avoid.
Calling malloc(0) merely produces a fiddly-to-deal-with corner case,
where it may succeed by returning 0.

realloc(something,0) is actually wrong.  If it returns 0 you don''t
know whether that means "I tried to allocate you a new 0-length block,
but failed, so your original block is still there" or "I have
successfully freed your old block; have this null pointer as an
answer".
> > Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
> > Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Acked-by: Ian Campbell <ian.campbell@citrix.com>
Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>

And put on the backport list.

Thanks,
Ian.

Ian Jackson

2013-Nov-25 13:55 UTC

head link

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Ian Jackson writes ("Re: [PATCH RESEND] libxl: Avoid realloc(,0) when
libxl__xs_directory returns empty list"):> Committed-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> And put on the backport list.
Now applied to 4.3.  Was not applicable to 4.2, 4.1.

Ian.

Reasonably Related Threads

Search for more apparently analagous threads

Xen devel - Apr 2013 - xl network-attach SEGV in 4.2 and 4.1

xl network-attach SEGV in 4.2 and 4.1

Re: xl network-attach SEGV in 4.2 and 4.1

Re: xl network-attach SEGV in 4.2 and 4.1

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

Re: xl network-attach SEGV in 4.2 and 4.1

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

Re: xl network-attach SEGV in 4.2 and 4.1 [and 1 more messages]

[PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Re: [PATCH RESEND] libxl: Avoid realloc(, 0) when libxl__xs_directory returns empty list

Reasonably Related Threads