Xen devel - Feb 2011 - [PATCH] qemu-xen: fix segfault with empty cdroms

When the cdrom 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 218f654..6aebb77 100644
--- a/hw/xen_disk.c
+++ b/hw/xen_disk.c
@@ -582,12 +582,13 @@ static int blk_init(struct XenDevice *xendev)
 {
     struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev);
     int mode, 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;

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

Stefano Stabellini writes ("[Xen-devel] [PATCH] qemu-xen: fix segfault with
empty cdroms"):
> When the cdrom is empty the params node in xenstore might be missing
> completely, cope with it instead of segfaulting.
...
>  	blkdev->params = xenstore_read_be_str(&blkdev->xendev,
"params");
> -        h = strchr(blkdev->params, '':'');
> +        if (blkdev->params != NULL)
> +            h = strchr(blkdev->params, '':'');
So blkdev->params may be 0.  In that case, we end up with:

              blkdev->fileproto = "<unset>";
              blkdev->filename  = blkdev->params;

so now ->filename may be 0.  Eventually,

              if (bdrv_open2(blkdev->bs, blkdev->filename, qflags,
                             bdrv_find_format(blkdev->fileproto)) != 0) {

Isn''t that going to crash ?

Perhaps a clause needs to be added to:

      /* do we have all we need? */
      if (blkdev->params == NULL ||
          blkdev->mode == NULL   ||
          blkdev->type == NULL   ||
          blkdev->dev == NULL)
          return -1;

Ian.

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

On Fri, 11 Feb 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("[Xen-devel] [PATCH] qemu-xen: fix segfault
with empty cdroms"):
> > When the cdrom is empty the params node in xenstore might be missing
> > completely, cope with it instead of segfaulting.
> ...
> >  	blkdev->params = xenstore_read_be_str(&blkdev->xendev,
"params");
> > -        h = strchr(blkdev->params, '':'');
> > +        if (blkdev->params != NULL)
> > +            h = strchr(blkdev->params, '':'');
> 
> So blkdev->params may be 0.  In that case, we end up with:
> 
>               blkdev->fileproto = "<unset>";
>               blkdev->filename  = blkdev->params;
> 
> so now ->filename may be 0.  Eventually,
> 
>               if (bdrv_open2(blkdev->bs, blkdev->filename, qflags,
>                              bdrv_find_format(blkdev->fileproto)) != 0)
{
> 
> Isn''t that going to crash ?
> 
> Perhaps a clause needs to be added to:
> 
>       /* do we have all we need? */
>       if (blkdev->params == NULL ||
>           blkdev->mode == NULL   ||
>           blkdev->type == NULL   ||
>           blkdev->dev == NULL)
>           return -1;
 

No need, in fact if blkdev->params is NULL we return -1 right here.

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

Stefano Stabellini writes ("Re: [Xen-devel] [PATCH] qemu-xen: fix segfault
with empty cdroms"):
> No need, in fact if blkdev->params is NULL we return -1 right here.
Oh, yes, sorry, I misread the earlier thing as assigning to params.
I''ll apply your patch.

Ian.

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