[PATCH kvmtool v3 0/2] Fix virtio/rng handling in low entropy situations

[PATCH kvmtool v3 0/2] Fix virtio/rng handling in low entropy situations

[Andre Przywara]

At the moment kvmtool uses the /dev/random device to back the randomness
provided by our virtio/rng implementation. We run it in non-blocking
mode, so are not affected by the nasty "can block indefinitely"
behaviour of that file. However:
- If /dev/random WOULD block, it returns EAGAIN, and we reflect that by
  adding 0 bytes of entropy to the virtio queue. However the virtio 1.x
  spec clearly says this is not allowed, and that we should always provide
  at least one random byte.
- If the guest is waiting for the random numbers, we still run into an
  effective blocking situation, because the buffer will only be filled
  very slowly, effectively stalling or blocking the guest. EDK II shows
  that behaviour, when servicing the EFI_RNG_PROTOCOL runtime service
  call, called by the kernel very early on boot.

Those two patches fix those problems, and allow to boot a Linux kernel
MUCH quicker when the host lacks good entropy sources. On a particular
system the kernel took 10 minutes to boot because of /dev/random
effectively blocking, this runs now at full speed.

The block is avoided by using /dev/urandom, there is a proper rabbit
hole in the internet out there why this is safe, even for cryptographic
applications.

Patch 2 aims to fix the corner case when the /dev/urandom read fails for
whatever reason: we just try once more in this case, since it should
only happen when the call is interrupted by a signal. This is not 100%
bullet proof, I am happy to hear any suggestions or whether we just
don't care about that very rare case.

Please have a look!

Cheers,
Andre

Changelog v2 ... v3:
- clamp request size on retry to 256 bytes

Changelog v1 ... v2:
- Drop O_NONBLOCK from the /dev/urandom open() call
- Drop block/unblock sequence after failed read, just retry once

Andre Przywara (2):
  virtio/rng: switch to using /dev/urandom
  virtio/rng: return at least one byte of entropy

 virtio/rng.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH kvmtool v3 1/2] virtio/rng: switch to using /dev/urandom

[Andre Przywara]

At the moment we use /dev/random as the backing device to provide random
numbers to our virtio-rng implementation. The downside of doing so is
that it may block indefinitely - or return EAGAIN repeatedly in our case.
On one headless system without ample noise sources (no keyboard, mouse,
or network traffic) I measured 30 seconds to gain one byte of randomness.
At the moment EDK II insists in waiting for all of the requsted random
bytes (for its EFI_RNG_PROTOCOL runtime service) to arrive, that held up
a Linux kernel boot for more than 10 minutes(!).

According to the Internet(TM), on Linux /dev/urandom provides the same
quality random numbers as /dev/random, it just does not block when the
entropy estimation algorithm suggests so. For all practical purposes the
recommendation is to just use /dev/urandom, QEMU did the switch as well
in 2019 [1].

Use /dev/urandom instead of /dev/random when opening the file descriptor
providing the randomness source for the virtio/rng implementation.
Due to a special behaviour documented on the urandom(4) manpage, a read
from /dev/urandom will never block, so we can drop the O_NONBLOCK flag.

[1] https://gitlab.com/qemu-project/qemu/-/commit/a2230bd778d8

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
 virtio/rng.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/virtio/rng.c b/virtio/rng.c
index 8f85d5ec1..e6e70ced3 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -166,7 +166,7 @@ int virtio_rng__init(struct kvm *kvm)
 	if (rdev == NULL)
 		return -ENOMEM;
 
-	rdev->fd = open("/dev/random", O_RDONLY | O_NONBLOCK);
+	rdev->fd = open("/dev/urandom", O_RDONLY);
 	if (rdev->fd < 0) {
 		r = rdev->fd;
 		goto cleanup;
-- 
2.25.1

[PATCH kvmtool v3 2/2] virtio/rng: return at least one byte of entropy

[Andre Przywara]

In contrast to the original v0.9 virtio spec (which was rather vague),
the virtio 1.0+ spec demands that a RNG request returns at least one
byte:
"The device MUST place one or more random bytes into the buffer, but it
MAY use less than the entire buffer length."

Our current implementation does not prevent returning zero bytes, which
upsets an assert in EDK II. /dev/urandom should always return at least
256 bytes of entropy, unless interrupted by a signal.

Repeat the read if that happens, and give up if that fails as well.
This makes sure we return some entropy and become spec compliant.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
Reported-by: Sami Mujawar <sami.mujawar@arm.com>
---
 virtio/rng.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/virtio/rng.c b/virtio/rng.c
index e6e70ced3..77a3a1138 100644
--- a/virtio/rng.c
+++ b/virtio/rng.c
@@ -61,13 +61,25 @@ static u64 get_host_features(struct kvm *kvm, void *dev)
 static bool virtio_rng_do_io_request(struct kvm *kvm, struct rng_dev *rdev, struct virt_queue *queue)
 {
 	struct iovec iov[VIRTIO_RNG_QUEUE_SIZE];
-	ssize_t len = 0;
+	ssize_t len;
 	u16 out, in, head;
 
 	head	= virt_queue__get_iov(queue, iov, &out, &in, kvm);
 	len	= readv(rdev->fd, iov, in);
-	if (len < 0 && errno == EAGAIN)
-		len = 0;
+	if (len < 0 && (errno == EAGAIN || errno == EINTR)) {
+		/*
+		 * The virtio 1.0 spec demands at least one byte of entropy,
+		 * so we cannot just return with 0 if something goes wrong.
+		 * The urandom(4) manpage mentions that a read from /dev/urandom
+		 * should always return at least 256 bytes of randomness, so
+		 * just retry here, with the requested size clamped to that
+		 * maximum, in case we were interrupted by a signal.
+		 */
+		iov[0].iov_len = min(iov[0].iov_len, 256UL);
+		len = readv(rdev->fd, iov, 1);
+		if (len < 1)
+			return false;
+	}
 
 	virt_queue__set_used_elem(queue, head, len);
 
-- 
2.25.1

Re: [PATCH kvmtool v3 0/2] Fix virtio/rng handling in low entropy situations

[Will Deacon]

On Wed, 24 May 2023 12:22:05 +0100, Andre Przywara wrote:
> At the moment kvmtool uses the /dev/random device to back the randomness
> provided by our virtio/rng implementation. We run it in non-blocking
> mode, so are not affected by the nasty "can block indefinitely"
> behaviour of that file. However:
> - If /dev/random WOULD block, it returns EAGAIN, and we reflect that by
>   adding 0 bytes of entropy to the virtio queue. However the virtio 1.x
>   spec clearly says this is not allowed, and that we should always provide
>   at least one random byte.
> - If the guest is waiting for the random numbers, we still run into an
>   effective blocking situation, because the buffer will only be filled
>   very slowly, effectively stalling or blocking the guest. EDK II shows
>   that behaviour, when servicing the EFI_RNG_PROTOCOL runtime service
>   call, called by the kernel very early on boot.
> 
> [...]

Applied to kvmtool (master), thanks!

[1/2] virtio/rng: switch to using /dev/urandom
      https://git.kernel.org/will/kvmtool/c/62ba372b0e67
[2/2] virtio/rng: return at least one byte of entropy
      https://git.kernel.org/will/kvmtool/c/bc23b9d9b152

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev