From: Akihiko Odaki <[email protected]>
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: [email protected]
Reviewed-by: Dmitry Osipenko <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
[ Marc-André - fix rebase conflict ]
Signed-off-by: Marc-André Lureau <[email protected]>
Signed-off-by: Akihiko Odaki <[email protected]>
Message-ID: <[email protected]>
---
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 b9bad27c97a8..2f60c72078b3 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -66,7 +66,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 718ba3039290..eac039c3c366 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;
}
@@ -720,6 +726,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));
@@ -745,11 +752,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);
@@ -760,6 +767,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) {
@@ -769,15 +777,15 @@ 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;
}
@@ -788,7 +796,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct
virtio_gpu_framebuffer *fb,
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;
@@ -1238,8 +1246,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,
--
2.55.0
