include/hw/virtio/virtio-gpu.h | 1 - hw/display/virtio-gpu.c | 27 +++++++++++++++++---------- 2 files changed, 17 insertions(+), 11 deletions(-)
virtio_gpu_do_set_scanout() validates the stride field of struct
virtio_gpu_framebuffer against the bytes_pp field, but bytes_pp in the
migration stream may be inconsistent with the format field, which
pixman_image_create_bits() uses when it accesses the framebuffer.
That validation is therefore incomplete.
To avoid the trouble of synchronizing the two fields, remove bytes_pp,
and always derive its value from format. Removing bytes_pp is safe
because no released version of QEMU uses its migrated value.
Fixes: 7b5574225429 ("hw/display: check frame buffer can hold blob")
Cc: qemu-stable@nongnu.org
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Based-on: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
("[PATCH] hw/display/virtio-gpu: validate stride against width on scanout")
---
include/hw/virtio/virtio-gpu.h | 1 -
hw/display/virtio-gpu.c | 27 +++++++++++++++++----------
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f69fc1946273..6f889ee84a16 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -65,7 +65,6 @@ struct virtio_gpu_simple_resource {
struct virtio_gpu_framebuffer {
pixman_format_code_t format;
- uint32_t bytes_pp;
uint32_t width, height;
uint32_t stride;
uint32_t offset;
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e00fb6effa50..660191d15f78 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -617,6 +617,11 @@ void virtio_gpu_update_scanout(VirtIOGPU *g,
scanout->fb = *fb;
}
+static uint32_t virtio_gpu_format_bytes_pp(pixman_format_code_t format)
+{
+ return DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
+}
+
static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
uint32_t scanout_id,
struct virtio_gpu_framebuffer *fb,
@@ -625,6 +630,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
uint32_t *error)
{
struct virtio_gpu_scanout *scanout;
+ uint32_t bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
uint8_t *data;
scanout = &g->parent_obj.scanout[scanout_id];
@@ -646,10 +652,10 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
return false;
}
- if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
+ if (fb->stride < (uint64_t)fb->width * bytes_pp) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: stride %u too small for width %u at %u bpp\n",
- __func__, fb->stride, fb->width, fb->bytes_pp);
+ __func__, fb->stride, fb->width, bytes_pp);
*error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
return false;
}
@@ -708,6 +714,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
struct virtio_gpu_simple_resource *res;
struct virtio_gpu_framebuffer fb = { 0 };
struct virtio_gpu_set_scanout ss;
+ uint32_t bytes_pp;
VIRTIO_GPU_FILL_CMD(ss);
virtio_gpu_bswap_32(&ss, sizeof(ss));
@@ -733,11 +740,11 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
}
fb.format = pixman_image_get_format(res->image);
- fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
+ bytes_pp = virtio_gpu_format_bytes_pp(fb.format);
fb.width = pixman_image_get_width(res->image);
fb.height = pixman_image_get_height(res->image);
fb.stride = pixman_image_get_stride(res->image);
- fb.offset = ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
+ fb.offset = ss.r.x * bytes_pp + ss.r.y * fb.stride;
virtio_gpu_do_set_scanout(g, ss.scanout_id,
&fb, res, &ss.r, &cmd->error);
@@ -748,6 +755,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
uint64_t blob_size)
{
uint64_t fbend;
+ uint32_t bytes_pp;
fb->format = virtio_gpu_get_pixman_format(ss->format);
if (!fb->format) {
@@ -757,19 +765,19 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
return false;
}
- fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8);
+ bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
fb->width = ss->width;
fb->height = ss->height;
fb->stride = ss->strides[0];
- if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
+ if (fb->stride < (uint64_t)fb->width * bytes_pp) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: stride %u too small for width %u at %u bpp\n",
- __func__, fb->stride, fb->width, fb->bytes_pp);
+ __func__, fb->stride, fb->width, bytes_pp);
return false;
}
- fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride;
+ fb->offset = ss->offsets[0] + ss->r.x * bytes_pp + ss->r.y * fb->stride;
fbend = fb->offset;
fbend += (uint64_t) fb->stride * ss->r.height;
@@ -1219,8 +1227,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = {
VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout),
VMSTATE_UINT32_TEST(fb.format, struct virtio_gpu_scanout,
scanout_vmstate_after_v2),
- VMSTATE_UINT32_TEST(fb.bytes_pp, struct virtio_gpu_scanout,
- scanout_vmstate_after_v2),
+ VMSTATE_UNUSED_TEST(scanout_vmstate_after_v2, 4),
VMSTATE_UINT32_TEST(fb.width, struct virtio_gpu_scanout,
scanout_vmstate_after_v2),
VMSTATE_UINT32_TEST(fb.height, struct virtio_gpu_scanout,
---
base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
change-id: 20260719-bpp-803a3544fb79
prerequisite-message-id: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
prerequisite-patch-id: 2c43dfa5d6a955e0fa3e3ca39ce3038968d85e7a
Best regards,
--
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>Hi
On Sun, Jul 19, 2026 at 3:36 PM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> virtio_gpu_do_set_scanout() validates the stride field of struct
> virtio_gpu_framebuffer against the bytes_pp field, but bytes_pp in the
> migration stream may be inconsistent with the format field, which
> pixman_image_create_bits() uses when it accesses the framebuffer.
> That validation is therefore incomplete.
>
> To avoid the trouble of synchronizing the two fields, remove bytes_pp,
> and always derive its value from format. Removing bytes_pp is safe
> because no released version of QEMU uses its migrated value.
>
> Fixes: 7b5574225429 ("hw/display: check frame buffer can hold blob")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> Based-on: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
> ("[PATCH] hw/display/virtio-gpu: validate stride against width on scanout")
rebased and applied on my next fix PR
thanks
> ---
> include/hw/virtio/virtio-gpu.h | 1 -
> hw/display/virtio-gpu.c | 27 +++++++++++++++++----------
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index f69fc1946273..6f889ee84a16 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -65,7 +65,6 @@ struct virtio_gpu_simple_resource {
>
> struct virtio_gpu_framebuffer {
> pixman_format_code_t format;
> - uint32_t bytes_pp;
> uint32_t width, height;
> uint32_t stride;
> uint32_t offset;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index e00fb6effa50..660191d15f78 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -617,6 +617,11 @@ void virtio_gpu_update_scanout(VirtIOGPU *g,
> scanout->fb = *fb;
> }
>
> +static uint32_t virtio_gpu_format_bytes_pp(pixman_format_code_t format)
> +{
> + return DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
> +}
> +
> static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> uint32_t scanout_id,
> struct virtio_gpu_framebuffer *fb,
> @@ -625,6 +630,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> uint32_t *error)
> {
> struct virtio_gpu_scanout *scanout;
> + uint32_t bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
> uint8_t *data;
>
> scanout = &g->parent_obj.scanout[scanout_id];
> @@ -646,10 +652,10 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> return false;
> }
>
> - if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
> + if (fb->stride < (uint64_t)fb->width * bytes_pp) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: stride %u too small for width %u at %u bpp\n",
> - __func__, fb->stride, fb->width, fb->bytes_pp);
> + __func__, fb->stride, fb->width, bytes_pp);
> *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> return false;
> }
> @@ -708,6 +714,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res;
> struct virtio_gpu_framebuffer fb = { 0 };
> struct virtio_gpu_set_scanout ss;
> + uint32_t bytes_pp;
>
> VIRTIO_GPU_FILL_CMD(ss);
> virtio_gpu_bswap_32(&ss, sizeof(ss));
> @@ -733,11 +740,11 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
> }
>
> fb.format = pixman_image_get_format(res->image);
> - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
> + bytes_pp = virtio_gpu_format_bytes_pp(fb.format);
> fb.width = pixman_image_get_width(res->image);
> fb.height = pixman_image_get_height(res->image);
> fb.stride = pixman_image_get_stride(res->image);
> - fb.offset = ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
> + fb.offset = ss.r.x * bytes_pp + ss.r.y * fb.stride;
>
> virtio_gpu_do_set_scanout(g, ss.scanout_id,
> &fb, res, &ss.r, &cmd->error);
> @@ -748,6 +755,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
> uint64_t blob_size)
> {
> uint64_t fbend;
> + uint32_t bytes_pp;
>
> fb->format = virtio_gpu_get_pixman_format(ss->format);
> if (!fb->format) {
> @@ -757,19 +765,19 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
> return false;
> }
>
> - fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8);
> + bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
> fb->width = ss->width;
> fb->height = ss->height;
> fb->stride = ss->strides[0];
>
> - if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
> + if (fb->stride < (uint64_t)fb->width * bytes_pp) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: stride %u too small for width %u at %u bpp\n",
> - __func__, fb->stride, fb->width, fb->bytes_pp);
> + __func__, fb->stride, fb->width, bytes_pp);
> return false;
> }
>
> - fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride;
> + fb->offset = ss->offsets[0] + ss->r.x * bytes_pp + ss->r.y * fb->stride;
>
> fbend = fb->offset;
> fbend += (uint64_t) fb->stride * ss->r.height;
> @@ -1219,8 +1227,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = {
> VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout),
> VMSTATE_UINT32_TEST(fb.format, struct virtio_gpu_scanout,
> scanout_vmstate_after_v2),
> - VMSTATE_UINT32_TEST(fb.bytes_pp, struct virtio_gpu_scanout,
> - scanout_vmstate_after_v2),
> + VMSTATE_UNUSED_TEST(scanout_vmstate_after_v2, 4),
> VMSTATE_UINT32_TEST(fb.width, struct virtio_gpu_scanout,
> scanout_vmstate_after_v2),
> VMSTATE_UINT32_TEST(fb.height, struct virtio_gpu_scanout,
>
> ---
> base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
> change-id: 20260719-bpp-803a3544fb79
> prerequisite-message-id: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
> prerequisite-patch-id: 2c43dfa5d6a955e0fa3e3ca39ce3038968d85e7a
>
> Best regards,
> --
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>
>
--
Marc-André Lureau
On 19/7/26 13:35, Akihiko Odaki wrote:
> virtio_gpu_do_set_scanout() validates the stride field of struct
> virtio_gpu_framebuffer against the bytes_pp field, but bytes_pp in the
> migration stream may be inconsistent with the format field, which
> pixman_image_create_bits() uses when it accesses the framebuffer.
> That validation is therefore incomplete.
>
> To avoid the trouble of synchronizing the two fields, remove bytes_pp,
> and always derive its value from format. Removing bytes_pp is safe
> because no released version of QEMU uses its migrated value.
>
> Fixes: 7b5574225429 ("hw/display: check frame buffer can hold blob")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> Based-on: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
> ("[PATCH] hw/display/virtio-gpu: validate stride against width on scanout")
> ---
> include/hw/virtio/virtio-gpu.h | 1 -
> hw/display/virtio-gpu.c | 27 +++++++++++++++++----------
> 2 files changed, 17 insertions(+), 11 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
On 7/19/26 14:35, Akihiko Odaki wrote:
> virtio_gpu_do_set_scanout() validates the stride field of struct
> virtio_gpu_framebuffer against the bytes_pp field, but bytes_pp in the
> migration stream may be inconsistent with the format field, which
> pixman_image_create_bits() uses when it accesses the framebuffer.
> That validation is therefore incomplete.
>
> To avoid the trouble of synchronizing the two fields, remove bytes_pp,
> and always derive its value from format. Removing bytes_pp is safe
> because no released version of QEMU uses its migrated value.
>
> Fixes: 7b5574225429 ("hw/display: check frame buffer can hold blob")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> Based-on: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
> ("[PATCH] hw/display/virtio-gpu: validate stride against width on scanout")
> ---
> include/hw/virtio/virtio-gpu.h | 1 -
> hw/display/virtio-gpu.c | 27 +++++++++++++++++----------
> 2 files changed, 17 insertions(+), 11 deletions(-)
>
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index f69fc1946273..6f889ee84a16 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -65,7 +65,6 @@ struct virtio_gpu_simple_resource {
>
> struct virtio_gpu_framebuffer {
> pixman_format_code_t format;
> - uint32_t bytes_pp;
> uint32_t width, height;
> uint32_t stride;
> uint32_t offset;
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index e00fb6effa50..660191d15f78 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -617,6 +617,11 @@ void virtio_gpu_update_scanout(VirtIOGPU *g,
> scanout->fb = *fb;
> }
>
> +static uint32_t virtio_gpu_format_bytes_pp(pixman_format_code_t format)
> +{
> + return DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
> +}
> +
> static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> uint32_t scanout_id,
> struct virtio_gpu_framebuffer *fb,
> @@ -625,6 +630,7 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> uint32_t *error)
> {
> struct virtio_gpu_scanout *scanout;
> + uint32_t bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
> uint8_t *data;
>
> scanout = &g->parent_obj.scanout[scanout_id];
> @@ -646,10 +652,10 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
> return false;
> }
>
> - if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
> + if (fb->stride < (uint64_t)fb->width * bytes_pp) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: stride %u too small for width %u at %u bpp\n",
> - __func__, fb->stride, fb->width, fb->bytes_pp);
> + __func__, fb->stride, fb->width, bytes_pp);
> *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> return false;
> }
> @@ -708,6 +714,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
> struct virtio_gpu_simple_resource *res;
> struct virtio_gpu_framebuffer fb = { 0 };
> struct virtio_gpu_set_scanout ss;
> + uint32_t bytes_pp;
>
> VIRTIO_GPU_FILL_CMD(ss);
> virtio_gpu_bswap_32(&ss, sizeof(ss));
> @@ -733,11 +740,11 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g,
> }
>
> fb.format = pixman_image_get_format(res->image);
> - fb.bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb.format), 8);
> + bytes_pp = virtio_gpu_format_bytes_pp(fb.format);
> fb.width = pixman_image_get_width(res->image);
> fb.height = pixman_image_get_height(res->image);
> fb.stride = pixman_image_get_stride(res->image);
> - fb.offset = ss.r.x * fb.bytes_pp + ss.r.y * fb.stride;
> + fb.offset = ss.r.x * bytes_pp + ss.r.y * fb.stride;
>
> virtio_gpu_do_set_scanout(g, ss.scanout_id,
> &fb, res, &ss.r, &cmd->error);
> @@ -748,6 +755,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
> uint64_t blob_size)
> {
> uint64_t fbend;
> + uint32_t bytes_pp;
>
> fb->format = virtio_gpu_get_pixman_format(ss->format);
> if (!fb->format) {
> @@ -757,19 +765,19 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
> return false;
> }
>
> - fb->bytes_pp = DIV_ROUND_UP(PIXMAN_FORMAT_BPP(fb->format), 8);
> + bytes_pp = virtio_gpu_format_bytes_pp(fb->format);
> fb->width = ss->width;
> fb->height = ss->height;
> fb->stride = ss->strides[0];
>
> - if (fb->stride < (uint64_t)fb->width * fb->bytes_pp) {
> + if (fb->stride < (uint64_t)fb->width * bytes_pp) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: stride %u too small for width %u at %u bpp\n",
> - __func__, fb->stride, fb->width, fb->bytes_pp);
> + __func__, fb->stride, fb->width, bytes_pp);
> return false;
> }
>
> - fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride;
> + fb->offset = ss->offsets[0] + ss->r.x * bytes_pp + ss->r.y * fb->stride;
>
> fbend = fb->offset;
> fbend += (uint64_t) fb->stride * ss->r.height;
> @@ -1219,8 +1227,7 @@ static const VMStateDescription vmstate_virtio_gpu_scanout = {
> VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout),
> VMSTATE_UINT32_TEST(fb.format, struct virtio_gpu_scanout,
> scanout_vmstate_after_v2),
> - VMSTATE_UINT32_TEST(fb.bytes_pp, struct virtio_gpu_scanout,
> - scanout_vmstate_after_v2),
> + VMSTATE_UNUSED_TEST(scanout_vmstate_after_v2, 4),
> VMSTATE_UINT32_TEST(fb.width, struct virtio_gpu_scanout,
> scanout_vmstate_after_v2),
> VMSTATE_UINT32_TEST(fb.height, struct virtio_gpu_scanout,
>
> ---
> base-commit: 499039798cdad7d86b787fec0eaf1da4151c0f05
> change-id: 20260719-bpp-803a3544fb79
> prerequisite-message-id: <20260715201055.2465320-1-marcandre.lureau@redhat.com>
> prerequisite-patch-id: 2c43dfa5d6a955e0fa3e3ca39ce3038968d85e7a
>
> Best regards,
> --
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
--
Best regards,
Dmitry
© 2016 - 2026 Red Hat, Inc.