Patchwork [v2] virtio-rng: Fix crash with non-default backend

login
register
mail settings
Submitter Cole Robinson
Date May 31, 2013, 6:12 p.m.
Message ID <bf4505014a0a941dbd3c62068f3cf2c496b69e6a.1370023944.git.crobinso@redhat.com>
Download mbox | patch
Permalink /patch/247997/
State New
Headers show

Comments

Cole Robinson - May 31, 2013, 6:12 p.m.
'default_backend' isn't always set, but 'rng' is, so use that.

$ ./x86_64-softmmu/qemu-system-x86_64 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-pci,rng=rng0
Segmentation fault (core dumped)

Regressed with virtio refactoring in 59ccd20a9ac719cff82180429458728f03ec612f

CC: qemu-stable@nongnu.org
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---

Notes:
    v2: Fix other instances in s390x code, pointed out by Frederic

 hw/s390x/s390-virtio-bus.c | 2 +-
 hw/s390x/virtio-ccw.c      | 2 +-
 hw/virtio/virtio-pci.c     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
Amit Shah - June 3, 2013, 7:02 a.m.
On (Fri) 31 May 2013 [14:12:48], Cole Robinson wrote:
> 'default_backend' isn't always set, but 'rng' is, so use that.
> 
> $ ./x86_64-softmmu/qemu-system-x86_64 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-pci,rng=rng0
> Segmentation fault (core dumped)
> 
> Regressed with virtio refactoring in 59ccd20a9ac719cff82180429458728f03ec612f
> 
> CC: qemu-stable@nongnu.org
> Signed-off-by: Cole Robinson <crobinso@redhat.com>
> ---
> 
> Notes:
>     v2: Fix other instances in s390x code, pointed out by Frederic

Acked-by: Amit Shah <amit.shah@redhat.com>

		Amit
Michael S. Tsirkin - June 3, 2013, 12:12 p.m.
On Fri, May 31, 2013 at 02:12:48PM -0400, Cole Robinson wrote:
> 'default_backend' isn't always set, but 'rng' is, so use that.
> 
> $ ./x86_64-softmmu/qemu-system-x86_64 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-pci,rng=rng0
> Segmentation fault (core dumped)
> 
> Regressed with virtio refactoring in 59ccd20a9ac719cff82180429458728f03ec612f
> 
> CC: qemu-stable@nongnu.org
> Signed-off-by: Cole Robinson <crobinso@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>

An unrelated comment:
we have many instances if the literal "rng"
string in code. They all must match otherwise
things crash.

So why not add a macro

#define VIRTIO_RNG_BACKEND_LINK "rng"

this way compiler checks that we don't make any typos.

> ---
> 
> Notes:
>     v2: Fix other instances in s390x code, pointed out by Frederic
> 
>  hw/s390x/s390-virtio-bus.c | 2 +-
>  hw/s390x/virtio-ccw.c      | 2 +-
>  hw/virtio/virtio-pci.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index a1cdfb0..207eb82 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -300,7 +300,7 @@ static int s390_virtio_rng_init(VirtIOS390Device *s390_dev)
>      }
>  
>      object_property_set_link(OBJECT(dev),
> -                             OBJECT(dev->vdev.conf.default_backend), "rng",
> +                             OBJECT(dev->vdev.conf.rng), "rng",
>                               NULL);
>  
>      return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 5f5e267..7dcd98d 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -744,7 +744,7 @@ static int virtio_ccw_rng_init(VirtioCcwDevice *ccw_dev)
>      }
>  
>      object_property_set_link(OBJECT(dev),
> -                             OBJECT(dev->vdev.conf.default_backend), "rng",
> +                             OBJECT(dev->vdev.conf.rng), "rng",
>                               NULL);
>  
>      return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 444b71a..b070b64 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1455,7 +1455,7 @@ static int virtio_rng_pci_init(VirtIOPCIProxy *vpci_dev)
>      }
>  
>      object_property_set_link(OBJECT(vrng),
> -                             OBJECT(vrng->vdev.conf.default_backend), "rng",
> +                             OBJECT(vrng->vdev.conf.rng), "rng",
>                               NULL);
>  
>      return 0;
> -- 
> 1.8.2.1
Michael Roth - June 12, 2013, 8:33 p.m.
On Fri, May 31, 2013 at 02:12:48PM -0400, Cole Robinson wrote:
> 'default_backend' isn't always set, but 'rng' is, so use that.
> 
> $ ./x86_64-softmmu/qemu-system-x86_64 -object rng-random,id=rng0,filename=/dev/random -device virtio-rng-pci,rng=rng0
> Segmentation fault (core dumped)
> 
> Regressed with virtio refactoring in 59ccd20a9ac719cff82180429458728f03ec612f
> 
> CC: qemu-stable@nongnu.org
> Signed-off-by: Cole Robinson <crobinso@redhat.com>

Tested-by: Michael Roth <mdroth@linux.vnet.ibm.com>

Looking to pull this in for 1.5.1

> ---
> 
> Notes:
>     v2: Fix other instances in s390x code, pointed out by Frederic
> 
>  hw/s390x/s390-virtio-bus.c | 2 +-
>  hw/s390x/virtio-ccw.c      | 2 +-
>  hw/virtio/virtio-pci.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index a1cdfb0..207eb82 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -300,7 +300,7 @@ static int s390_virtio_rng_init(VirtIOS390Device *s390_dev)
>      }
> 
>      object_property_set_link(OBJECT(dev),
> -                             OBJECT(dev->vdev.conf.default_backend), "rng",
> +                             OBJECT(dev->vdev.conf.rng), "rng",
>                               NULL);
> 
>      return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index 5f5e267..7dcd98d 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -744,7 +744,7 @@ static int virtio_ccw_rng_init(VirtioCcwDevice *ccw_dev)
>      }
> 
>      object_property_set_link(OBJECT(dev),
> -                             OBJECT(dev->vdev.conf.default_backend), "rng",
> +                             OBJECT(dev->vdev.conf.rng), "rng",
>                               NULL);
> 
>      return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 444b71a..b070b64 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1455,7 +1455,7 @@ static int virtio_rng_pci_init(VirtIOPCIProxy *vpci_dev)
>      }
> 
>      object_property_set_link(OBJECT(vrng),
> -                             OBJECT(vrng->vdev.conf.default_backend), "rng",
> +                             OBJECT(vrng->vdev.conf.rng), "rng",
>                               NULL);
> 
>      return 0;
> -- 
> 1.8.2.1
> 
>
Anthony Liguori - June 21, 2013, 3:34 p.m.
Applied.  Thanks.

Regards,

Anthony Liguori

Patch

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index a1cdfb0..207eb82 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -300,7 +300,7 @@  static int s390_virtio_rng_init(VirtIOS390Device *s390_dev)
     }
 
     object_property_set_link(OBJECT(dev),
-                             OBJECT(dev->vdev.conf.default_backend), "rng",
+                             OBJECT(dev->vdev.conf.rng), "rng",
                              NULL);
 
     return s390_virtio_device_init(s390_dev, VIRTIO_DEVICE(vdev));
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 5f5e267..7dcd98d 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -744,7 +744,7 @@  static int virtio_ccw_rng_init(VirtioCcwDevice *ccw_dev)
     }
 
     object_property_set_link(OBJECT(dev),
-                             OBJECT(dev->vdev.conf.default_backend), "rng",
+                             OBJECT(dev->vdev.conf.rng), "rng",
                              NULL);
 
     return virtio_ccw_device_init(ccw_dev, VIRTIO_DEVICE(vdev));
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 444b71a..b070b64 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1455,7 +1455,7 @@  static int virtio_rng_pci_init(VirtIOPCIProxy *vpci_dev)
     }
 
     object_property_set_link(OBJECT(vrng),
-                             OBJECT(vrng->vdev.conf.default_backend), "rng",
+                             OBJECT(vrng->vdev.conf.rng), "rng",
                              NULL);
 
     return 0;