[PATCH 1/1] virtio: rng: disallow multiple device registrations, fixes crashes

[PATCH 1/1] virtio: rng: disallow multiple device registrations, fixes crashes

[Amit Shah]

The code currently only supports one virtio-rng device at a time.
Invoking guests with multiple devices causes the guest to blow up.

Check if we've already registered and initialised the driver.  Also
cleanup in case of registration errors or hot-unplug so that a new
device can be used.

Reported-by: Peter Krempa <pkrempa@redhat.com>
Reported-by: <yunzheng@redhat.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---

Also valid for stable?

 drivers/char/hw_random/virtio-rng.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 10fd71c..6bf4d47 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -92,14 +92,22 @@ static int probe_common(struct virtio_device *vdev)
 {
 	int err;
 
+	if (vq) {
+		/* We only support one device for now */
+		return -EBUSY;
+	}
 	/* We expect a single virtqueue. */
 	vq = virtio_find_single_vq(vdev, random_recv_done, "input");
-	if (IS_ERR(vq))
-		return PTR_ERR(vq);
+	if (IS_ERR(vq)) {
+		err = PTR_ERR(vq);
+		vq = NULL;
+		return err;
+	}
 
 	err = hwrng_register(&virtio_hwrng);
 	if (err) {
 		vdev->config->del_vqs(vdev);
+		vq = NULL;
 		return err;
 	}
 
@@ -112,6 +120,7 @@ static void remove_common(struct virtio_device *vdev)
 	busy = false;
 	hwrng_unregister(&virtio_hwrng);
 	vdev->config->del_vqs(vdev);
+	vq = NULL;
 }
 
 static int virtrng_probe(struct virtio_device *vdev)
-- 
1.8.1.2

Re: [PATCH 1/1] virtio: rng: disallow multiple device registrations, fixes crashes

[Rusty Russell]

Amit Shah <amit.shah@redhat.com> writes:
> The code currently only supports one virtio-rng device at a time.
> Invoking guests with multiple devices causes the guest to blow up.
>
> Check if we've already registered and initialised the driver.  Also
> cleanup in case of registration errors or hot-unplug so that a new
> device can be used.
>
> Reported-by: Peter Krempa <pkrempa@redhat.com>
> Reported-by: <yunzheng@redhat.com>
> Signed-off-by: Amit Shah <amit.shah@redhat.com>
> ---
>
> Also valid for stable?

Yes.  We could fix virtio-rng to allow multiple rngs, but of course it
will fail anyway since hwrng wants unique names.  And changing the name
to be virtio-%u will probably break things, for no real upside.

Applied, and Cc:stable added.

Thanks,
Rusty.

Re: [PATCH 1/1] virtio: rng: disallow multiple device registrations, fixes crashes

[Amit Shah]

(CC'ing lkml and hpa for thoughts on multiple active hwrng devices)

On (Thu) 07 Mar 2013 [12:06:31], Rusty Russell wrote:
> Amit Shah <amit.shah@redhat.com> writes:
> > The code currently only supports one virtio-rng device at a time.
> > Invoking guests with multiple devices causes the guest to blow up.
> >
> > Check if we've already registered and initialised the driver.  Also
> > cleanup in case of registration errors or hot-unplug so that a new
> > device can be used.
> >
> > Reported-by: Peter Krempa <pkrempa@redhat.com>
> > Reported-by: <yunzheng@redhat.com>
> > Signed-off-by: Amit Shah <amit.shah@redhat.com>
> > ---
> >
> > Also valid for stable?
> 
> Yes.  We could fix virtio-rng to allow multiple rngs, but of course it
> will fail anyway since hwrng wants unique names.  And changing the name
> to be virtio-%u will probably break things, for no real upside.

The hwrng interface also sources its input from one active device at a
time, and that's selectable via a sysfs interface.

If we extend the hwrng interface to source from multiple devices at
the same time, and taking in whatever it gets from whichever device
has data to give, wlil having multiple virtio devices make sense.

But, several active hwrng devices has its own set of problems: on need
for entropy, which device do you ask from?  Just asking all devices
for precious entropy, when only one could give out all of it is not
productive (or secure?).

> Applied, and Cc:stable added.

Thanks!

		Amit