Hi
On Sun, Jul 26, 2026 at 11:03 AM Akihiko Odaki
<[email protected]> wrote:
>
> On 2026/07/25 21:27, [email protected] wrote:
> > From: Marc-André Lureau <[email protected]>
> >
> > virtio_gpu_scanout_blob_to_fb() computes the framebuffer offset from
> > guest-controlled offsets[0], r.x, r.y and stride using uint32_t
> > arithmetic. When the sum exceeds UINT32_MAX, silent wraparound lets
> > the guest steer the scanout to an arbitrary in-bounds region of the
> > blob instead of the intended rectangle.
> >
> > Compute the offset in uint64_t, reject values exceeding UINT32_MAX
> > (the width of fb->offset), and only store into fb->offset once both
> > range checks pass.
>
> The calculation of offset can still overflow uint64_t. All supported
> formats use four bytes per pixel, so
> the maximum mathematical value is:
>
> UINT32_MAX + 4 * UINT32_MAX + UINT32_MAX * UINT32_MAX
>
> which exceeds UINT64_MAX.
The code rejects stride > INT_MAX, that should make the sum maximum
below UINT64_MAX
>
> This is reachable through virgl_cmd_set_scanout_blob(), whose
> rectangle checks use wrapping uint32_t additions. For example:
>
> width = height = 16
> stride = r.y = blob_size = UINT32_MAX
> r.x = 0x7fffffff
> r.width = 0x80000001
> r.height = 1
> offsets[0] = 3
>
> Both rectangle sums wrap to zero, while the mathematical offset is
> exactly 2^64. Consequently, offset becomes zero, fbend becomes
> M, and both bounds checks pass. The non-virgl caller retroactively
> rejects this geometry. The virgl caller fails to reject it.
That's a separate fix though, open a security issue?
>
> >
> > Fixes: 32db3c63ae11 ("virtio-gpu: Add virtio_gpu_set_scanout_blob")
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3871
> > Based-on: <[email protected]>
> > ("[PATCH] hw/display/virtio-gpu: Remove the bytes_pp field")
>
> Please remove Based-on: from the patch message when sending a pull
> request. You can put the tag in the footer. For example:
>
> https://patchew.org/QEMU/[email protected]/
>
> Regards,
> Akihiko Odaki
>
> > Reported-by: Cyber_black <[email protected]>
> > Signed-off-by: Marc-André Lureau <[email protected]>> ---
> > v2:
> > - no OOB, dropping CVE, make commit message adjustments
> > ---
> > hw/display/virtio-gpu.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> > index b21ac8dace08..bc45cfb194f0 100644
> > --- a/hw/display/virtio-gpu.c
> > +++ b/hw/display/virtio-gpu.c
> > @@ -785,7 +785,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct
> > virtio_gpu_framebuffer *fb,
> > struct virtio_gpu_set_scanout_blob *ss,
> > uint64_t blob_size)
> > {
> > - uint64_t fbend;
> > + uint64_t fbend, offset;
> > uint32_t bytes_pp;
> >
> > fb->format = virtio_gpu_get_pixman_format(ss->format);
> > @@ -815,18 +815,20 @@ bool virtio_gpu_scanout_blob_to_fb(struct
> > virtio_gpu_framebuffer *fb,
> > return false;
> > }
> >
> > - fb->offset = ss->offsets[0] + ss->r.x * bytes_pp + ss->r.y *
> > fb->stride;
> > + offset = (uint64_t)ss->offsets[0] + (uint64_t)ss->r.x * bytes_pp +
> > + (uint64_t)ss->r.y * fb->stride;
> >
> > - fbend = fb->offset;
> > - fbend += (uint64_t) fb->stride * ss->r.height;
> > + fbend = offset + (uint64_t)fb->stride * ss->r.height;
> >
> > - if (fbend > blob_size) {
> > + if (offset > UINT32_MAX || fbend > blob_size) {
> > qemu_log_mask(LOG_GUEST_ERROR,
> > - "%s: fb end out of range\n",
> > + "%s: invalid fb bounds\n",
> > __func__);
> > return false;
> > }
> >
> > + fb->offset = offset;
> > +
> > return true;
> > }
> >
>
>
