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-devel

[Xen-devel] [PATCH] fix invalid free segfault and use-after-free in libx

To: Xen Devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] fix invalid free segfault and use-after-free in libxl_device_disk_list()
From: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>
Date: Fri, 13 Aug 2010 16:57:44 +0100
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
Delivery-date: Fri, 13 Aug 2010 09:03:21 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
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>

diff -r dc335ebde3b5 tools/libxl/libxl.c
--- a/tools/libxl/libxl.c       Thu Aug 12 18:03:23 2010 +0100
+++ b/tools/libxl/libxl.c       Fri Aug 13 17:01:34 2010 +0100
@@ -1303,17 +1303,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) {
+    for (i; i < nb; i++) {
         if ( disks->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);
 }
@@ -2435,7 +2435,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);
@@ -2450,9 +2450,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;
@@ -2470,9 +2470,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;
@@ -2552,6 +2552,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-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

<Prev in Thread] Current Thread [Next in Thread>