Re: [PATCH 10/11] virtio: rng: Check length before copying

["Pierre-Clément Tosi" <ptosi@google.com>]

Hi,

On Thu, Mar 31, 2022 at 10:09:48AM +0000, Andrew Scull wrote:
> Check the length of data written by the device is consistent with the
> size of the buffers to avoid out-of-bounds memory accesses in case
> values aren't consistent.
> 
> Signed-off-by: Andrew Scull <ascull@google.com>
> Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>  drivers/virtio/virtio_rng.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
> index 9314c0a03e..b85545c2ee 100644
> --- a/drivers/virtio/virtio_rng.c
> +++ b/drivers/virtio/virtio_rng.c
> @@ -41,6 +41,9 @@ static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
>  		while (!virtqueue_get_buf(priv->rng_vq, &rsize))
>  			;
>  
> +		if (rsize > sg.length)
> +			return -EIO;
> +

Although this patch addresses a legitimate concern, could we instead aim for
strengthening the lower-level virtio building blocks (e.g. virtqueue_get_buf())
so that higher-level virtio device drivers such as virtio-{rng,console,net,...}
don't have to be littered with checks of this nature? Could this be achieved by
using the shadow copy introduced in [PATCH 03/11]?

>  		memcpy(ptr, buf, rsize);
>  		len -= rsize;
>  		ptr += rsize;
> -- 
> 2.35.1.1094.g7c7d902a7c-goog
> 

Thanks,

-- 
Pierre