debuggers.hg

changeset 22042:1382296539fc

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@citrix.com>
Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
author Gianni Tedesco <gianni.tedesco@citrix.com>
date Mon Aug 16 13:37:58 2010 +0100 (2010-08-16)
parents be41e8ee8052
children def814437d42
files tools/libxl/libxl.c
line diff
     1.1 --- a/tools/libxl/libxl.c	Mon Aug 16 13:33:54 2010 +0100
     1.2 +++ b/tools/libxl/libxl.c	Mon Aug 16 13:37:58 2010 +0100
     1.3 @@ -1315,17 +1315,17 @@ static char ** libxl_build_device_model_
     1.4          flexarray_set(dm_args, num++, "xenfv");
     1.5  
     1.6      disks = libxl_device_disk_list(libxl_gc_owner(gc), info->domid, &nb);
     1.7 -    for (; nb > 0; --nb, ++disks) {
     1.8 -        if ( disks->is_cdrom ) {
     1.9 +    for (i; i < nb; i++) {
    1.10 +        if ( disks[i].is_cdrom ) {
    1.11              flexarray_set(dm_args, num++, "-cdrom");
    1.12 -            flexarray_set(dm_args, num++, disks->physpath);
    1.13 +            flexarray_set(dm_args, num++, disks[i].physpath);
    1.14          }else{
    1.15 -            flexarray_set(dm_args, num++, libxl_sprintf(gc, "-%s", disks->virtpath));
    1.16 -            flexarray_set(dm_args, num++, disks->physpath);
    1.17 +            flexarray_set(dm_args, num++, libxl_sprintf(gc, "-%s", disks[i].virtpath));
    1.18 +            flexarray_set(dm_args, num++, disks[i].physpath);
    1.19          }
    1.20      }
    1.21 +    /* FIXME: leaks disk paths */
    1.22      free(disks);
    1.23 -
    1.24      flexarray_set(dm_args, num++, NULL);
    1.25      return (char **) flexarray_contents(dm_args);
    1.26  }
    1.27 @@ -2462,7 +2462,7 @@ libxl_device_disk *libxl_device_disk_lis
    1.28      char *be_path_tap, *be_path_vbd;
    1.29      libxl_device_disk *dend, *disks, *ret = NULL;
    1.30      char **b, **l = NULL;
    1.31 -    unsigned int numl;
    1.32 +    unsigned int numl, len;
    1.33      char *type;
    1.34  
    1.35      be_path_vbd = libxl_sprintf(&gc, "%s/backend/vbd/%d", libxl_xs_get_dompath(&gc, 0), domid);
    1.36 @@ -2477,9 +2477,9 @@ libxl_device_disk *libxl_device_disk_lis
    1.37          for (; disks < dend; ++disks, ++l) {
    1.38              disks->backend_domid = 0;
    1.39              disks->domid = domid;
    1.40 -            disks->physpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/params", be_path_vbd, *l));
    1.41 +            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, "%s/%s/params", be_path_vbd, *l), &len);
    1.42              libxl_string_to_phystype(ctx, libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/type", be_path_vbd, *l)), &(disks->phystype));
    1.43 -            disks->virtpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/dev", be_path_vbd, *l));
    1.44 +            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, "%s/%s/dev", be_path_vbd, *l), &len);
    1.45              disks->unpluggable = atoi(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/removable", be_path_vbd, *l)));
    1.46              if (!strcmp(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/mode", be_path_vbd, *l)), "w"))
    1.47                  disks->readwrite = 1;
    1.48 @@ -2497,9 +2497,9 @@ libxl_device_disk *libxl_device_disk_lis
    1.49          for (dend = ret + *num; disks < dend; ++disks, ++l) {
    1.50              disks->backend_domid = 0;
    1.51              disks->domid = domid;
    1.52 -            disks->physpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/params", be_path_tap, *l));
    1.53 +            disks->physpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, "%s/%s/params", be_path_tap, *l), &len);
    1.54              libxl_string_to_phystype(ctx, libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/type", be_path_tap, *l)), &(disks->phystype));
    1.55 -            disks->virtpath = libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/dev", be_path_tap, *l));
    1.56 +            disks->virtpath = xs_read(ctx->xsh, XBT_NULL, libxl_sprintf(&gc, "%s/%s/dev", be_path_tap, *l), &len);
    1.57              disks->unpluggable = atoi(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/removable", be_path_tap, *l)));
    1.58              if (!strcmp(libxl_xs_read(&gc, XBT_NULL, libxl_sprintf(&gc, "%s/%s/mode", be_path_tap, *l)), "w"))
    1.59                  disks->readwrite = 1;
    1.60 @@ -2579,6 +2579,7 @@ int libxl_cdrom_insert(libxl_ctx *ctx, u
    1.61          libxl_device_disk_add(ctx, stubdomid, disk);
    1.62          disk->domid = domid;
    1.63      }
    1.64 +    /* FIXME: leaks disk paths */
    1.65      free(disks);
    1.66      return 0;
    1.67  }