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) {
+        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",
-- 
2.51.1

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

On 11/25/25 14:54, Alex Bennée wrote:
> 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?

The second check catches integer overflow for huge mblob.offset that is
u64 coming from guest. This wasn't caught before, we missed validation
of the offset value.

-- 
Best regards,
Dmitry