| Message ID | 1351272088-7942-3-git-send-email-aliguori@us.ibm.com |
|---|---|
| State | New |
| Headers | show |
Il 26/10/2012 19:21, Anthony Liguori ha scritto: > This allows you to specify: > > $ qemu -device virtio-rng-pci > > And things will Just Work with a reasonable default. > > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> > --- > hw/virtio-pci.c | 13 +++++++++++++ > hw/virtio-rng.h | 2 ++ > 2 files changed, 15 insertions(+) > > diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c > index 0dc2a06..cfdb779 100644 > --- a/hw/virtio-pci.c > +++ b/hw/virtio-pci.c > @@ -885,6 +885,19 @@ static int virtio_rng_init_pci(PCIDevice *pci_dev) > VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); > VirtIODevice *vdev; > > + if (proxy->rng.rng == NULL) { > + proxy->rng.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); > + > + object_property_add_child(OBJECT(pci_dev), > + "default-backend", > + OBJECT(proxy->rng.default_backend), > + NULL); > + > + object_property_set_link(OBJECT(pci_dev), > + OBJECT(proxy->rng.default_backend), > + "rng", NULL); > + } > + > vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng); > if (!vdev) { > return -1; > diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h > index fbb0104..63ddb96 100644 > --- a/hw/virtio-rng.h > +++ b/hw/virtio-rng.h > @@ -13,12 +13,14 @@ > #define _QEMU_VIRTIO_RNG_H > > #include "qemu/rng.h" > +#include "qemu/rng-random.h" > > /* The Virtio ID for the virtio rng device */ > #define VIRTIO_ID_RNG 4 > > struct VirtIORNGConf { > RngBackend *rng; > + RndRandom *default_backend; > }; > > #endif > NACK. Starting a guest that runs rngd (or just a malicious guest) will completely deprive the host of entropy. If you make the default /dev/hwrng, however, that would be ok. Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 26/10/2012 19:21, Anthony Liguori ha scritto: >> This allows you to specify: >> >> $ qemu -device virtio-rng-pci >> >> And things will Just Work with a reasonable default. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> hw/virtio-pci.c | 13 +++++++++++++ >> hw/virtio-rng.h | 2 ++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >> index 0dc2a06..cfdb779 100644 >> --- a/hw/virtio-pci.c >> +++ b/hw/virtio-pci.c >> @@ -885,6 +885,19 @@ static int virtio_rng_init_pci(PCIDevice *pci_dev) >> VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); >> VirtIODevice *vdev; >> >> + if (proxy->rng.rng == NULL) { >> + proxy->rng.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); >> + >> + object_property_add_child(OBJECT(pci_dev), >> + "default-backend", >> + OBJECT(proxy->rng.default_backend), >> + NULL); >> + >> + object_property_set_link(OBJECT(pci_dev), >> + OBJECT(proxy->rng.default_backend), >> + "rng", NULL); >> + } >> + >> vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng); >> if (!vdev) { >> return -1; >> diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h >> index fbb0104..63ddb96 100644 >> --- a/hw/virtio-rng.h >> +++ b/hw/virtio-rng.h >> @@ -13,12 +13,14 @@ >> #define _QEMU_VIRTIO_RNG_H >> >> #include "qemu/rng.h" >> +#include "qemu/rng-random.h" >> >> /* The Virtio ID for the virtio rng device */ >> #define VIRTIO_ID_RNG 4 >> >> struct VirtIORNGConf { >> RngBackend *rng; >> + RndRandom *default_backend; >> }; >> >> #endif >> > > NACK. Starting a guest that runs rngd (or just a malicious guest) will > completely deprive the host of entropy. That's why this is a separate series... Still don't understand what the default entropy source should be. > If you make the default /dev/hwrng, however, that would be ok. /dev/hwrng may be (and stay) empty which seems unfortunate. I was thinking /dev/urandom would be a good pragmatic choice though. Regards, Anthony Liguori > > Paolo
Il 26/10/2012 20:59, Paolo Bonzini ha scritto: > Il 26/10/2012 19:21, Anthony Liguori ha scritto: >> This allows you to specify: >> >> $ qemu -device virtio-rng-pci >> >> And things will Just Work with a reasonable default. >> >> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >> --- >> hw/virtio-pci.c | 13 +++++++++++++ >> hw/virtio-rng.h | 2 ++ >> 2 files changed, 15 insertions(+) >> >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >> index 0dc2a06..cfdb779 100644 >> --- a/hw/virtio-pci.c >> +++ b/hw/virtio-pci.c >> @@ -885,6 +885,19 @@ static int virtio_rng_init_pci(PCIDevice *pci_dev) >> VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); >> VirtIODevice *vdev; >> >> + if (proxy->rng.rng == NULL) { >> + proxy->rng.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); >> + >> + object_property_add_child(OBJECT(pci_dev), >> + "default-backend", >> + OBJECT(proxy->rng.default_backend), >> + NULL); >> + >> + object_property_set_link(OBJECT(pci_dev), >> + OBJECT(proxy->rng.default_backend), >> + "rng", NULL); >> + } >> + >> vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng); >> if (!vdev) { >> return -1; >> diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h >> index fbb0104..63ddb96 100644 >> --- a/hw/virtio-rng.h >> +++ b/hw/virtio-rng.h >> @@ -13,12 +13,14 @@ >> #define _QEMU_VIRTIO_RNG_H >> >> #include "qemu/rng.h" >> +#include "qemu/rng-random.h" >> >> /* The Virtio ID for the virtio rng device */ >> #define VIRTIO_ID_RNG 4 >> >> struct VirtIORNGConf { >> RngBackend *rng; >> + RndRandom *default_backend; >> }; >> >> #endif >> > > NACK. Starting a guest that runs rngd (or just a malicious guest) will > completely deprive the host of entropy. > > If you make the default /dev/hwrng, however, that would be ok. Also, does this break non-Linux? What if the default was changed to /dev/hwrng but an older Linux distro didn't have the device file at all? Paolo
Paolo Bonzini <pbonzini@redhat.com> writes: > Il 26/10/2012 20:59, Paolo Bonzini ha scritto: >> Il 26/10/2012 19:21, Anthony Liguori ha scritto: >>> This allows you to specify: >>> >>> $ qemu -device virtio-rng-pci >>> >>> And things will Just Work with a reasonable default. >>> >>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> >>> --- >>> hw/virtio-pci.c | 13 +++++++++++++ >>> hw/virtio-rng.h | 2 ++ >>> 2 files changed, 15 insertions(+) >>> >>> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c >>> index 0dc2a06..cfdb779 100644 >>> --- a/hw/virtio-pci.c >>> +++ b/hw/virtio-pci.c >>> @@ -885,6 +885,19 @@ static int virtio_rng_init_pci(PCIDevice *pci_dev) >>> VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); >>> VirtIODevice *vdev; >>> >>> + if (proxy->rng.rng == NULL) { >>> + proxy->rng.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); >>> + >>> + object_property_add_child(OBJECT(pci_dev), >>> + "default-backend", >>> + OBJECT(proxy->rng.default_backend), >>> + NULL); >>> + >>> + object_property_set_link(OBJECT(pci_dev), >>> + OBJECT(proxy->rng.default_backend), >>> + "rng", NULL); >>> + } >>> + >>> vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng); >>> if (!vdev) { >>> return -1; >>> diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h >>> index fbb0104..63ddb96 100644 >>> --- a/hw/virtio-rng.h >>> +++ b/hw/virtio-rng.h >>> @@ -13,12 +13,14 @@ >>> #define _QEMU_VIRTIO_RNG_H >>> >>> #include "qemu/rng.h" >>> +#include "qemu/rng-random.h" >>> >>> /* The Virtio ID for the virtio rng device */ >>> #define VIRTIO_ID_RNG 4 >>> >>> struct VirtIORNGConf { >>> RngBackend *rng; >>> + RndRandom *default_backend; >>> }; >>> >>> #endif >>> >> >> NACK. Starting a guest that runs rngd (or just a malicious guest) will >> completely deprive the host of entropy. >> >> If you make the default /dev/hwrng, however, that would be ok. > > Also, does this break non-Linux? It should fail gracefully. If you do: qemu -device virtio-rng-pci Before this series you'd get: qemu: Invalid value for parameter 'rng', expects a valid object Now on !Linux you would get: qemu -device virtio-rng-pci qemu: Failed to open /dev/random So it's still a failure, just a different message. But this does suggest that we shouldn't add it to the default machine on !Linux because we don't want the default machine failing. Perhaps we can find better default backends on !Linux... > What if the default was changed to /dev/hwrng but an older Linux > distro didn't have the device file at all? It would throw an error gracefully. Regards, Anthony Liguori > > Paolo
Il 26/10/2012 21:51, Anthony Liguori ha scritto: >> > If you make the default /dev/hwrng, however, that would be ok. > /dev/hwrng may be (and stay) empty which seems unfortunate. Unfortunate, but at least not wrong. > I was thinking /dev/urandom would be a good pragmatic choice though. No. /dev/urandom is actively wrong because it provides the guest with the illusion of an infinite source of entropy, while the guest is really being fed with an infinite source of pseudo-random numbers. /dev/random as a default is bad because on hosts without neither hwrng nor rdrand it completely depletes the host's entropy pool. Thus it denies access to entropy to other programs running in the host. I thought /dev/random with some throttling would be good, especially if somehow the guest can be told to run rngd in skip-test mode, e.g. via a virtio-rng feature bit. Peter's last messages make me wonder if this is correct. If it is, the throttling can be implemented either in QEMU or outside it (via a daemon that speaks the same protocol as egd). /dev/random might be good in the special case where rngd is being run in the host, and there is an hwrng or rdrand to feed rngd. In this case the guest can also be run in skip-test mode. However, I don't have a machine at hand (it's Friday evening here) to test whether rngd could keep up, or a malicious guest would instead also deplete the host's entropy tool too badly. /dev/null makes the guest behave exactly as if no virtio-rng-pci is present, so it is at least not wrong. rdrand and /dev/hwrng seem to be the best choice at least to me. Peter seemed to agree initially, then said "This is surreal. Output from /dev/hwrng turns into output for /dev/random... it us guaranteed worse; period, end of story". I'm confused. I hope the above is not too inaccurate and at least a decent way to reset the discussion. Paolo
Il 26/10/2012 22:16, Anthony Liguori ha scritto: > So it's still a failure, just a different message. > > But this does suggest that we shouldn't add it to the default machine on > !Linux because we don't want the default machine failing. We need to add it to all machines, because the machine signature should not depend on the host OS. However, it does suggest we need a null backend as a fallback. > Perhaps we can find better default backends on !Linux... Yes, rdrand would be one. >> > What if the default was changed to /dev/hwrng but an older Linux >> > distro didn't have the device file at all? > It would throw an error gracefully. Perhaps the default backend can be created with a function that tries multiple things and at worst settles on rng-null. Paolo
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c index 0dc2a06..cfdb779 100644 --- a/hw/virtio-pci.c +++ b/hw/virtio-pci.c @@ -885,6 +885,19 @@ static int virtio_rng_init_pci(PCIDevice *pci_dev) VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, pci_dev); VirtIODevice *vdev; + if (proxy->rng.rng == NULL) { + proxy->rng.default_backend = RNG_RANDOM(object_new(TYPE_RNG_RANDOM)); + + object_property_add_child(OBJECT(pci_dev), + "default-backend", + OBJECT(proxy->rng.default_backend), + NULL); + + object_property_set_link(OBJECT(pci_dev), + OBJECT(proxy->rng.default_backend), + "rng", NULL); + } + vdev = virtio_rng_init(&pci_dev->qdev, &proxy->rng); if (!vdev) { return -1; diff --git a/hw/virtio-rng.h b/hw/virtio-rng.h index fbb0104..63ddb96 100644 --- a/hw/virtio-rng.h +++ b/hw/virtio-rng.h @@ -13,12 +13,14 @@ #define _QEMU_VIRTIO_RNG_H #include "qemu/rng.h" +#include "qemu/rng-random.h" /* The Virtio ID for the virtio rng device */ #define VIRTIO_ID_RNG 4 struct VirtIORNGConf { RngBackend *rng; + RndRandom *default_backend; }; #endif
This allows you to specify: $ qemu -device virtio-rng-pci And things will Just Work with a reasonable default. Signed-off-by: Anthony Liguori <aliguori@us.ibm.com> --- hw/virtio-pci.c | 13 +++++++++++++ hw/virtio-rng.h | 2 ++ 2 files changed, 15 insertions(+)