On Wed, Apr 19, 2023 at 06:05:26PM +0100, Andre Przywara wrote:
> 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@xxxxxxx>
> Reported-by: Sami Mujawar <sami.mujawar@xxxxxxx>
> ---
> virtio/rng.c | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/virtio/rng.c b/virtio/rng.c
> index e6e70ced3..d5959d358 100644
> --- a/virtio/rng.c
> +++ b/virtio/rng.c
> @@ -66,8 +66,18 @@ static bool virtio_rng_do_io_request(struct kvm *kvm, struct rng_dev *rdev, stru
>
> 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
I guess that's implied, but strictly speaking the manpage only states that
reads of <=256 bytes succeed. Larger reads may return an error again or
(if you read the man naively) zero bytes. We could increase the chance of
this succeeding by setting in = 1 and iov_len = min(iov_len, 256)
Thanks,
Jean
> + * just retry here in case we were interrupted by a signal.
> + */
> + len = readv(rdev->fd, iov, in);
> + if (len < 1)
> + return false;
> + }
>
> virt_queue__set_used_elem(queue, head, len);
>
> --
> 2.25.1
>
