Patchwork xen_disk: cope with missing xenstore "params" node

login
register
mail settings
Submitter Stefano Stabellini
Date Feb. 11, 2011, 12:38 p.m.
Message ID <alpine.DEB.2.00.1102111232290.2826@kaball-desktop>
Download mbox | patch
Permalink /patch/82751/
State New
Headers show

Comments

Stefano Stabellini - Feb. 11, 2011, 12:38 p.m.
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.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Kevin Wolf - Feb. 11, 2011, 12:49 p.m.
Am 11.02.2011 13:38, schrieb Stefano Stabellini:
> 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.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> 
> 
> diff --git a/hw/xen_disk.c b/hw/xen_disk.c
> index 134ac33..e553c4c 100644
> --- a/hw/xen_disk.c
> +++ b/hw/xen_disk.c
> @@ -577,12 +577,13 @@ 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;
> +    char *h = NULL;
>  
>      /* read xenstore entries */
>      if (blkdev->params == NULL) {
>  	blkdev->params = xenstore_read_be_str(&blkdev->xendev, "params");
> -        h = strchr(blkdev->params, ':');
> +        if (blkdev->params != NULL)
> +            h = strchr(blkdev->params, ':');

The coding style requires braces here.

>  	if (h != NULL) {
>  	    blkdev->fileproto = blkdev->params;
>  	    blkdev->filename  = h+1;

Let me add some more context:

    if (h != NULL) {
        blkdev->fileproto = blkdev->params;
        blkdev->filename  = h+1;
        *h = 0;
    } else {
        blkdev->fileproto = "<unset>";
        blkdev->filename  = blkdev->params;
    }

So in the NULL case we now have blkdev->filename = NULL. Doesn't this
just move the crash a few lines downwards when bdrv_open() tries to use
NULL as its filename?

Kevin

Patch

diff --git a/hw/xen_disk.c b/hw/xen_disk.c
index 134ac33..e553c4c 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -577,12 +577,13 @@  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;
+    char *h = NULL;
 
     /* read xenstore entries */
     if (blkdev->params == 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;