WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-changelog

[Xen-changelog] [xen-unstable] tools/libxl: fix memory management bugs i

To: xen-changelog@xxxxxxxxxxxxxxxxxxx
Subject: [Xen-changelog] [xen-unstable] tools/libxl: fix memory management bugs in libxl_device_disk_list()
From: Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>
Date: Wed, 18 Aug 2010 07:00:54 -0700
Delivery-date: Wed, 18 Aug 2010 07:05:28 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-changelog-request@lists.xensource.com?subject=help>
List-id: BK change log <xen-changelog.lists.xensource.com>
List-post: <mailto:xen-changelog@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-changelog>, <mailto:xen-changelog-request@lists.xensource.com?subject=unsubscribe>
Reply-to: xen-devel@xxxxxxxxxxxxxxxxxxx
Sender: xen-changelog-bounces@xxxxxxxxxxxxxxxxxxx
# HG changeset patch
# User Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
# Date 1281962278 -3600
# Node ID 1382296539fc73e259e96b1d8d59ab285a24712d
# Parent  be41e8ee8052a6d2a56161112375988a44f33ad2
tools/libxl: fix memory management bugs in libxl_device_disk_list()

fix invalid free segfault and use-after-free in libxl_device_disk_list()

Gah, libxl_device_disk_list() is returning a lot of pointers to free'd
data. Fix that by replacing libxl_xs_read() with xs_read() in line with
the policy.

Also fix a segfault caused by an erroneous free of the last disk-list
array element rather than the first one. This was causing xl create to
segfault when using the new qemu-dm code-base. Fix that and add a
comment about the fact that this libxl API requires a corresponding
libxl_device_disk_free() function.

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
---
 tools/libxl/libxl.c |   27 ++++++++++++++-------------
 1 files changed, 14 insertions(+), 13 deletions(-)

diff -r be41e8ee8052 -r 1382296539fc tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Mon Aug 16 13:33:54 2010 +0100
+++ b/tools/libxl/libxl.c       Mon Aug 16 13:37:58 2010 +0100
@@ -1315,17 +1315,17 @@ static char ** libxl_build_device_model_
         flexarray_set(dm_args, num++, "xenfv");
 
     disks = libxl_device_disk_list(libxl_gc_owner(gc), info->domid, &nb);
-    for (; nb > 0; --nb, ++disks) {
-        if ( disks->is_cdrom ) {
+    for (i; i < nb; i++) {
+        if ( disks[i].is_cdrom ) {
             flexarray_set(dm_args, num++, "-cdrom");
-            flexarray_set(dm_args, num++, disks->physpath);
+            flexarray_set(dm_args, num++, disks[i].physpath);
         }else{
-            flexarray_set(dm_args, num++, libxl_sprintf(gc, "-%s", 
disks->virtpath));
-            flexarray_set(dm_args, num++, disks->physpath);
-        }
-    }
+            flexarray_set(dm_args, num++, libxl_sprintf(gc, "-%s", 
disks[i].virtpath));
+            flexarray_set(dm_args, num++, disks[i].physpath);
+        }
+    }
+    /* FIXME: leaks disk paths */
     free(disks);
-
     flexarray_set(dm_args, num++, NULL);
     return (char **) flexarray_contents(dm_args);
 }
@@ -2462,7 +2462,7 @@ libxl_device_disk *libxl_device_disk_lis
     char *be_path_tap, *be_path_vbd;
     libxl_device_disk *dend, *disks, *ret = NULL;
     char **b, **l = NULL;
-    unsigned int numl;
+    unsigned int numl, len;
     char *type;
 
     be_path_vbd = libxl_sprintf(&gc, "%s/backend/vbd/%d", 
libxl_xs_get_dompath(&gc, 0), domid);
@@ -2477,9 +2477,9 @@ libxl_device_disk *libxl_device_disk_lis
         for (; disks < dend; ++disks, ++l) {
             disks->backend_domid = 0;
             disks->domid = domid;
-            disks->physpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/params", be_path_vbd, *l));
+            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/params", be_path_vbd, *l), &len);
             libxl_string_to_phystype(ctx, libxl_xs_read(&gc, XBT_NULL, 
libxl_sprintf(&gc, "%s/%s/type", be_path_vbd, *l)), &(disks->phystype));
-            disks->virtpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/dev", be_path_vbd, *l));
+            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/dev", be_path_vbd, *l), &len);
             disks->unpluggable = atoi(libxl_xs_read(&gc, XBT_NULL, 
libxl_sprintf(&gc, "%s/%s/removable", be_path_vbd, *l)));
             if (!strcmp(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/mode", be_path_vbd, *l)), "w"))
                 disks->readwrite = 1;
@@ -2497,9 +2497,9 @@ libxl_device_disk *libxl_device_disk_lis
         for (dend = ret + *num; disks < dend; ++disks, ++l) {
             disks->backend_domid = 0;
             disks->domid = domid;
-            disks->physpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/params", be_path_tap, *l));
+            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/params", be_path_tap, *l), &len);
             libxl_string_to_phystype(ctx, libxl_xs_read(&gc, XBT_NULL, 
libxl_sprintf(&gc, "%s/%s/type", be_path_tap, *l)), &(disks->phystype));
-            disks->virtpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/dev", be_path_tap, *l));
+            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/dev", be_path_tap, *l), &len);
             disks->unpluggable = atoi(libxl_xs_read(&gc, XBT_NULL, 
libxl_sprintf(&gc, "%s/%s/removable", be_path_tap, *l)));
             if (!strcmp(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, 
"%s/%s/mode", be_path_tap, *l)), "w"))
                 disks->readwrite = 1;
@@ -2579,6 +2579,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
         libxl_device_disk_add(ctx, stubdomid, disk);
         disk->domid = domid;
     }
+    /* FIXME: leaks disk paths */
     free(disks);
     return 0;
 }

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

<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-changelog] [xen-unstable] tools/libxl: fix memory management bugs in libxl_device_disk_list(), Xen patchbot-unstable <=