Re: [RFC PATCH v4 2/7] virtio-gpu: Validate hostmem mapping offset

["Alex Bennée" <alex.bennee@linaro.org>]

Dmitry Osipenko <dmitry.osipenko@collabora.com> writes:

> Check hostmem mapping boundaries originated from guest.
>
> Suggested-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> Signed-off-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> ---
>  hw/display/virtio-gpu-virgl.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index a6860f63b563..2224f59cf5d7 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -126,6 +126,14 @@ virtio_gpu_virgl_map_resource_blob(VirtIOGPU *g,
>          return -EOPNOTSUPP;
>      }
>  
> +    if (offset + res->base.blob_size > b->conf.hostmem ||
> +        offset + res->base.blob_size < offset) {

This second check seems weird. offset + blob_size could only every be
smaller than offset if blob_size was negative. I feel we should have
caught that earlier if it can happen.

Are we trying to catch an overflow here?

> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: failed to map virgl resource: invalid offset\n",
> +                      __func__);
> +        return -EINVAL;
> +    }
> +
>      ret = virgl_renderer_resource_map(res->base.resource_id, &data, &size);
>      if (ret) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map virgl resource: %s\n",

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro