On Tue, 23 May 2023 11:45:03 +0100
Will Deacon <will@xxxxxxxxxx> wrote:
Hi Will,
> On Thu, Apr 20, 2023 at 02:46:27PM +0100, Jean-Philippe Brucker wrote:
> > 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)
>
> Andre -- do you plan to respin with Jean's suggestion above?
Yes, sorry, I read too much into the suggestion at first, so it got pushed
off the table.
But reading that again now and looking at the code I realise that it's
indeed easy to implement. I will post something shortly.
Thanks,
Andre.
