Re: [PATCH v2 2/2] virtio: rng: ensure reads happen after successful probe

From: Kees Cook
Date: Mon Jul 07 2014 - 00:39:04 EST


On Fri, Jul 4, 2014 at 10:34 PM, Amit Shah <amit.shah@xxxxxxxxxx> wrote:
> The hwrng core asks for random data in the hwrng_register() call itself
> from commit d9e7972619. This doesn't play well with virtio -- the
> DRIVER_OK bit is only set by virtio core on a successful probe, and
> we're not yet out of our probe routine when this call is made. This
> causes the host to not acknowledge any requests we put in the virtqueue,
> and the insmod or kernel boot process just waits for data to arrive from
> the host, which never happens.

Doesn't this mean that virtio-rng won't ever contribute entropy to the system?

-Kees

>
> CC: Kees Cook <keescook@xxxxxxxxxxxx>
> CC: Jason Cooper <jason@xxxxxxxxxxxxxx>
> CC: Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx>
> CC: <stable@xxxxxxxxxxxxxxx> # For v3.15+
> Signed-off-by: Amit Shah <amit.shah@xxxxxxxxxx>
> ---
> drivers/char/hw_random/core.c | 6 ++++++
> drivers/char/hw_random/virtio-rng.c | 10 ++++++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
> index df95e2f..50f5b76 100644
> --- a/drivers/char/hw_random/core.c
> +++ b/drivers/char/hw_random/core.c
> @@ -68,6 +68,12 @@ static void add_early_randomness(struct hwrng *rng)
> unsigned char bytes[16];
> int bytes_read;
>
> + /*
> + * Currently only virtio-rng cannot return data during device
> + * probe, and that's handled in virtio-rng.c itself. If there
> + * are more such devices, this call to rng_get_data can be
> + * made conditional here instead of doing it per-device.
> + */
> bytes_read = rng_get_data(rng, bytes, sizeof(bytes), 1);
> if (bytes_read > 0)
> add_device_randomness(bytes, bytes_read);
> diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
> index f3e7150..157f27c 100644
> --- a/drivers/char/hw_random/virtio-rng.c
> +++ b/drivers/char/hw_random/virtio-rng.c
> @@ -38,6 +38,8 @@ struct virtrng_info {
> int index;
> };
>
> +bool probe_done;
> +
> static void random_recv_done(struct virtqueue *vq)
> {
> struct virtrng_info *vi = vq->vdev->priv;
> @@ -67,6 +69,13 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait)
> int ret;
> struct virtrng_info *vi = (struct virtrng_info *)rng->priv;
>
> + /*
> + * Don't ask host for data till we're setup. This call can
> + * happen during hwrng_register(), after commit d9e7972619.
> + */
> + if (unlikely(!probe_done))
> + return 0;
> +
> if (!vi->busy) {
> vi->busy = true;
> init_completion(&vi->have_data);
> @@ -137,6 +146,7 @@ static int probe_common(struct virtio_device *vdev)
> return err;
> }
>
> + probe_done = true;
> return 0;
> }
>
> --
> 1.9.3
>



--
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/