Xen devel - Jun 2011 - [PATCH v3] xen_disk: cope with missing xenstore "params" node

From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

When disk is a cdrom and the drive is empty the "params" node in
xenstore might be missing completely: cope with it instead of
segfaulting.


Updated in v2:

- actually removed the strchr(blkdev->params, '':'') that
caused the
segfault;

- free all the allocated strings from xenstore before returning;


Updated in v3:

- set blkdev fields to NULL after free''ing them.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/xen_disk.c |   34 +++++++++++++++++++++++++++-------
 1 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 096d1c9..faaa065 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -616,12 +616,14 @@ static int blk_init(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     int index, qflags, have_barriers, info = 0;
-    char *h;
 
     /* read xenstore entries */
     if (blkdev->params == NULL) {
+        char *h = NULL;
         blkdev->params = xenstore_read_be_str(&blkdev->xendev,
"params");
-        h = strchr(blkdev->params, '':'');
+        if (blkdev->params != NULL) {
+            h = strchr(blkdev->params, '':'');
+        }
         if (h != NULL) {
             blkdev->fileproto = blkdev->params;
             blkdev->filename  = h+1;
@@ -649,7 +651,7 @@ static int blk_init(struct XenDevice *xendev)
         blkdev->mode == NULL   ||
         blkdev->type == NULL   ||
         blkdev->dev == NULL) {
-        return -1;
+        goto out_error;
     }
 
     /* read-only ? */
@@ -672,10 +674,15 @@ static int blk_init(struct XenDevice *xendev)
         /* setup via xenbus -> create new block driver instance */
         xen_be_printf(&blkdev->xendev, 2, "create new bdrv (xenbus
setup)\n");
         blkdev->bs = bdrv_new(blkdev->dev);
-        if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
-                      bdrv_find_whitelisted_format(blkdev->fileproto)) != 0)
{
-            bdrv_delete(blkdev->bs);
-            return -1;
+        if (blkdev->bs) {
+            if (bdrv_open(blkdev->bs, blkdev->filename, qflags,
+                        bdrv_find_whitelisted_format(blkdev->fileproto)) !=
0) {
+                bdrv_delete(blkdev->bs);
+                blkdev->bs = NULL;
+            }
+        }
+        if (!blkdev->bs) {
+            goto out_error;
         }
     } else {
         /* setup via qemu cmdline -> already setup for us */
@@ -704,6 +711,19 @@ static int blk_init(struct XenDevice *xendev)
     xenstore_write_be_int(&blkdev->xendev, "sectors",
                           blkdev->file_size / blkdev->file_blk);
     return 0;
+
+out_error:
+    qemu_free(blkdev->params);
+    blkdev->params = NULL;
+    qemu_free(blkdev->mode);
+    blkdev->mode = NULL;
+    qemu_free(blkdev->type);
+    blkdev->type = NULL;
+    qemu_free(blkdev->dev);
+    blkdev->dev = NULL;
+    qemu_free(blkdev->devtype);
+    blkdev->devtype = NULL;
+    return -1;
 }
 
 static int blk_connect(struct XenDevice *xendev)
-- 
1.7.2.3


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

On 06/27/2011 05:10 PM, stefano.stabellini@eu.citrix.com
wrote:
> From: Stefano Stabellini<stefano.stabellini@eu.citrix.com>
>
> When disk is a cdrom and the drive is empty the "params" node in
> xenstore might be missing completely: cope with it instead of
> segfaulting.
>
>
> Updated in v2:
>
> - actually removed the strchr(blkdev->params, '':'')
that caused the
> segfault;
>
> - free all the allocated strings from xenstore before returning;
>
>
> Updated in v3:
>
> - set blkdev fields to NULL after free''ing them.
Thanks, applied to xen-next branch.


Alex


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