* [Qemu-devel] [PULL v2 0/6] vga patch queue
@ 2016-06-06 7:06 Gerd Hoffmann
2016-06-06 7:06 ` [Qemu-devel] [PULL 1/6] virtio-gpu: fix scanout rectangles Gerd Hoffmann
` (6 more replies)
0 siblings, 7 replies; 10+ messages in thread
From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Hi,
vga patch queue brings some security fixes for vmware vga
and live migration support (in 2d mode) for virtio-gpu.
v2: add qemu-stable to cc for the security fixes.
please pull,
Gerd
The following changes since commit 2c107d7684f9e3c4db4780d0756bbf35b06aec07:
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-06-02 14:26:57 +0100)
are available in the git repository at:
git://git.kraxel.org/qemu tags/pull-vga-20160606-1
for you to fetch changes up to 0c244e50ee12311037efd507ee37df0e846e4a18:
virtio-gpu: add live migration support (2016-06-06 09:04:34 +0200)
----------------------------------------------------------------
virtio-gpu: scanout fix, live migration support
vmsvga: security fixes
----------------------------------------------------------------
Gerd Hoffmann (6):
virtio-gpu: fix scanout rectangles
vmsvga: move fifo sanity checks to vmsvga_fifo_length
vmsvga: add more fifo checks
vmsvga: shadow fifo registers
vmsvga: don't process more than 1024 fifo commands at once
virtio-gpu: add live migration support
hw/display/virtio-gpu-3d.c | 2 +-
hw/display/virtio-gpu.c | 202 +++++++++++++++++++++++++++++++++++++++--
hw/display/virtio-vga.c | 12 +++
hw/display/vmware_vga.c | 78 ++++++++--------
include/hw/virtio/virtio-gpu.h | 4 +-
5 files changed, 252 insertions(+), 46 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PULL 1/6] virtio-gpu: fix scanout rectangles 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length Gerd Hoffmann ` (5 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin Commit "ca58b45 ui/virtio-gpu: add and use qemu_create_displaysurface_pixman" breaks scanouts which use a region of the underlying resource only. So, we need another way to handle the underlying issue. Lets create a new pixman image, grab a reference on the pixman providing the underlying storage, hook up a destroy callback which releases the reference. That way regions work again and releasing the backing storage should still be impossible thanks to the extra reference we are holding. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-id: 1464597655-26341-1-git-send-email-kraxel@redhat.com --- hw/display/virtio-gpu.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index f3b0f14..a836e35 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -495,6 +495,11 @@ static void virtio_gpu_resource_flush(VirtIOGPU *g, pixman_region_fini(&flush_region); } +static void virtio_unref_resource(pixman_image_t *image, void *data) +{ + pixman_image_unref(data); +} + static void virtio_gpu_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { @@ -571,8 +576,15 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g, != ((uint8_t *)pixman_image_get_data(res->image) + offset) || scanout->width != ss.r.width || scanout->height != ss.r.height) { + pixman_image_t *rect; + void *ptr = (uint8_t *)pixman_image_get_data(res->image) + offset; + rect = pixman_image_create_bits(format, ss.r.width, ss.r.height, ptr, + pixman_image_get_stride(res->image)); + pixman_image_ref(res->image); + pixman_image_set_destroy_function(rect, virtio_unref_resource, + res->image); /* realloc the surface ptr */ - scanout->ds = qemu_create_displaysurface_pixman(res->image); + scanout->ds = qemu_create_displaysurface_pixman(rect); if (!scanout->ds) { cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; return; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 1/6] virtio-gpu: fix scanout rectangles Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 3/6] vmsvga: add more fifo checks Gerd Hoffmann ` (4 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, qemu-stable, P J P Sanity checks are applied when the fifo is enabled by the guest (SVGA_REG_CONFIG_DONE write). Which doesn't help much if the guest changes the fifo registers afterwards. Move the checks to vmsvga_fifo_length so they are done each time qemu is about to read from the fifo. Fixes: CVE-2016-4454 Cc: qemu-stable@nongnu.org Cc: P J P <ppandit@redhat.com> Reported-by: 李强 <liqiang6-s@360.cn> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464592161-18348-2-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 0c63fa8..63a7c05 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -555,6 +555,21 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) if (!s->config || !s->enable) { return 0; } + + /* Check range and alignment. */ + if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { + return 0; + } + if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { + return 0; + } + if (CMD(max) > SVGA_FIFO_SIZE) { + return 0; + } + if (CMD(max) < CMD(min) + 10 * 1024) { + return 0; + } + num = CMD(next_cmd) - CMD(stop); if (num < 0) { num += CMD(max) - CMD(min); @@ -1005,19 +1020,6 @@ static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value) case SVGA_REG_CONFIG_DONE: if (value) { s->fifo = (uint32_t *) s->fifo_ptr; - /* Check range and alignment. */ - if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { - break; - } - if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { - break; - } - if (CMD(max) > SVGA_FIFO_SIZE) { - break; - } - if (CMD(max) < CMD(min) + 10 * 1024) { - break; - } vga_dirty_log_stop(&s->vga); } s->config = !!value; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 3/6] vmsvga: add more fifo checks 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 1/6] virtio-gpu: fix scanout rectangles Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 4/6] vmsvga: shadow fifo registers Gerd Hoffmann ` (3 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, qemu-stable, P J P Make sure all fifo ptrs are within range. Fixes: CVE-2016-4454 Cc: qemu-stable@nongnu.org Cc: P J P <ppandit@redhat.com> Reported-by: 李强 <liqiang6-s@360.cn> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464592161-18348-3-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 63a7c05..a26e62e 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -563,7 +563,10 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { return 0; } - if (CMD(max) > SVGA_FIFO_SIZE) { + if (CMD(max) > SVGA_FIFO_SIZE || + CMD(min) >= SVGA_FIFO_SIZE || + CMD(stop) >= SVGA_FIFO_SIZE || + CMD(next_cmd) >= SVGA_FIFO_SIZE) { return 0; } if (CMD(max) < CMD(min) + 10 * 1024) { -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 4/6] vmsvga: shadow fifo registers 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann ` (2 preceding siblings ...) 2016-06-06 7:06 ` [Qemu-devel] [PULL 3/6] vmsvga: add more fifo checks Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 5/6] vmsvga: don't process more than 1024 fifo commands at once Gerd Hoffmann ` (2 subsequent siblings) 6 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, qemu-stable, P J P The fifo is normal ram. So kvm vcpu threads and qemu iothread can access the fifo in parallel without syncronization. Which in turn implies we can't use the fifo pointers in-place because the guest can try changing them underneath us. So add shadows for them, to make sure the guest can't modify them after we've applied sanity checks. Fixes: CVE-2016-4454 Cc: qemu-stable@nongnu.org Cc: P J P <ppandit@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464592161-18348-4-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 57 ++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index a26e62e..de2567b 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -66,17 +66,11 @@ struct vmsvga_state_s { uint8_t *fifo_ptr; unsigned int fifo_size; - union { - uint32_t *fifo; - struct QEMU_PACKED { - uint32_t min; - uint32_t max; - uint32_t next_cmd; - uint32_t stop; - /* Add registers here when adding capabilities. */ - uint32_t fifo[0]; - } *cmd; - }; + uint32_t *fifo; + uint32_t fifo_min; + uint32_t fifo_max; + uint32_t fifo_next; + uint32_t fifo_stop; #define REDRAW_FIFO_LEN 512 struct vmsvga_rect_s { @@ -198,7 +192,7 @@ enum { */ SVGA_FIFO_MIN = 0, SVGA_FIFO_MAX, /* The distance from MIN to MAX must be at least 10K */ - SVGA_FIFO_NEXT_CMD, + SVGA_FIFO_NEXT, SVGA_FIFO_STOP, /* @@ -546,8 +540,6 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s, } #endif -#define CMD(f) le32_to_cpu(s->cmd->f) - static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) { int num; @@ -556,38 +548,44 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) return 0; } + s->fifo_min = le32_to_cpu(s->fifo[SVGA_FIFO_MIN]); + s->fifo_max = le32_to_cpu(s->fifo[SVGA_FIFO_MAX]); + s->fifo_next = le32_to_cpu(s->fifo[SVGA_FIFO_NEXT]); + s->fifo_stop = le32_to_cpu(s->fifo[SVGA_FIFO_STOP]); + /* Check range and alignment. */ - if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { + if ((s->fifo_min | s->fifo_max | s->fifo_next | s->fifo_stop) & 3) { return 0; } - if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { + if (s->fifo_min < sizeof(uint32_t) * 4) { return 0; } - if (CMD(max) > SVGA_FIFO_SIZE || - CMD(min) >= SVGA_FIFO_SIZE || - CMD(stop) >= SVGA_FIFO_SIZE || - CMD(next_cmd) >= SVGA_FIFO_SIZE) { + if (s->fifo_max > SVGA_FIFO_SIZE || + s->fifo_min >= SVGA_FIFO_SIZE || + s->fifo_stop >= SVGA_FIFO_SIZE || + s->fifo_next >= SVGA_FIFO_SIZE) { return 0; } - if (CMD(max) < CMD(min) + 10 * 1024) { + if (s->fifo_max < s->fifo_min + 10 * 1024) { return 0; } - num = CMD(next_cmd) - CMD(stop); + num = s->fifo_next - s->fifo_stop; if (num < 0) { - num += CMD(max) - CMD(min); + num += s->fifo_max - s->fifo_min; } return num >> 2; } static inline uint32_t vmsvga_fifo_read_raw(struct vmsvga_state_s *s) { - uint32_t cmd = s->fifo[CMD(stop) >> 2]; + uint32_t cmd = s->fifo[s->fifo_stop >> 2]; - s->cmd->stop = cpu_to_le32(CMD(stop) + 4); - if (CMD(stop) >= CMD(max)) { - s->cmd->stop = s->cmd->min; + s->fifo_stop += 4; + if (s->fifo_stop >= s->fifo_max) { + s->fifo_stop = s->fifo_min; } + s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop); return cmd; } @@ -607,7 +605,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) len = vmsvga_fifo_length(s); while (len > 0) { /* May need to go back to the start of the command if incomplete */ - cmd_start = s->cmd->stop; + cmd_start = s->fifo_stop; switch (cmd = vmsvga_fifo_read(s)) { case SVGA_CMD_UPDATE: @@ -766,7 +764,8 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s) break; rewind: - s->cmd->stop = cmd_start; + s->fifo_stop = cmd_start; + s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop); break; } } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 5/6] vmsvga: don't process more than 1024 fifo commands at once 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann ` (3 preceding siblings ...) 2016-06-06 7:06 ` [Qemu-devel] [PULL 4/6] vmsvga: shadow fifo registers Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support Gerd Hoffmann 2016-06-06 14:17 ` [Qemu-devel] [PULL v2 0/6] vga patch queue Peter Maydell 6 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, qemu-stable, P J P vmsvga_fifo_run is called in regular intervals (on each display update) and will resume where it left off. So we can simply exit the loop, without having to worry about how processing will continue. Fixes: CVE-2016-4453 Cc: qemu-stable@nongnu.org Cc: P J P <ppandit@redhat.com> Reported-by: 李强 <liqiang6-s@360.cn> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464592161-18348-5-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index de2567b..e51a05e 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -597,13 +597,13 @@ static inline uint32_t vmsvga_fifo_read(struct vmsvga_state_s *s) static void vmsvga_fifo_run(struct vmsvga_state_s *s) { uint32_t cmd, colour; - int args, len; + int args, len, maxloop = 1024; int x, y, dx, dy, width, height; struct vmsvga_cursor_definition_s cursor; uint32_t cmd_start; len = vmsvga_fifo_length(s); - while (len > 0) { + while (len > 0 && --maxloop > 0) { /* May need to go back to the start of the command if incomplete */ cmd_start = s->fifo_stop; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann ` (4 preceding siblings ...) 2016-06-06 7:06 ` [Qemu-devel] [PULL 5/6] vmsvga: don't process more than 1024 fifo commands at once Gerd Hoffmann @ 2016-06-06 7:06 ` Gerd Hoffmann 2016-06-14 13:26 ` Paolo Bonzini 2016-06-06 14:17 ` [Qemu-devel] [PULL v2 0/6] vga patch queue Peter Maydell 6 siblings, 1 reply; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-06 7:06 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, Michael S. Tsirkin Store some additional state for cursor and resource backing storage, so we can write out and reload things. Implement vmsave+vmload for 2d mode. Continue blocking live migration in 3d/virgl mode. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464009727-7753-1-git-send-email-kraxel@redhat.com --- hw/display/virtio-gpu-3d.c | 2 +- hw/display/virtio-gpu.c | 188 +++++++++++++++++++++++++++++++++++++++-- hw/display/virtio-vga.c | 12 +++ include/hw/virtio/virtio-gpu.h | 4 +- 4 files changed, 198 insertions(+), 8 deletions(-) diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index 433bf93..29918a0 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -284,7 +284,7 @@ static void virgl_resource_attach_backing(VirtIOGPU *g, VIRTIO_GPU_FILL_CMD(att_rb); trace_virtio_gpu_cmd_res_back_attach(att_rb.resource_id); - ret = virtio_gpu_create_mapping_iov(&att_rb, cmd, &res_iovs); + ret = virtio_gpu_create_mapping_iov(&att_rb, cmd, NULL, &res_iovs); if (ret != 0) { cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; return; diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index a836e35..136c095 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -22,6 +22,8 @@ #include "qemu/log.h" #include "qapi/error.h" +#define VIRTIO_GPU_VM_VERSION 1 + static struct virtio_gpu_simple_resource* virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id); @@ -94,7 +96,7 @@ static void update_cursor_data_virgl(VirtIOGPU *g, static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor) { struct virtio_gpu_scanout *s; - bool move = cursor->hdr.type != VIRTIO_GPU_CMD_MOVE_CURSOR; + bool move = cursor->hdr.type == VIRTIO_GPU_CMD_MOVE_CURSOR; if (cursor->pos.scanout_id >= g->conf.max_outputs) { return; @@ -107,7 +109,7 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor) move ? "move" : "update", cursor->resource_id); - if (move) { + if (!move) { if (!s->current_cursor) { s->current_cursor = cursor_alloc(64, 64); } @@ -120,6 +122,11 @@ static void update_cursor(VirtIOGPU *g, struct virtio_gpu_update_cursor *cursor) g, s, cursor->resource_id); } dpy_cursor_define(s->con, s->current_cursor); + + s->cursor = *cursor; + } else { + s->cursor.pos.x = cursor->pos.x; + s->cursor.pos.y = cursor->pos.y; } dpy_mouse_set(s->con, cursor->pos.x, cursor->pos.y, cursor->resource_id ? 1 : 0); @@ -602,7 +609,7 @@ static void virtio_gpu_set_scanout(VirtIOGPU *g, int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab, struct virtio_gpu_ctrl_command *cmd, - struct iovec **iov) + uint64_t **addr, struct iovec **iov) { struct virtio_gpu_mem_entry *ents; size_t esize, s; @@ -628,10 +635,16 @@ int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab, } *iov = g_malloc0(sizeof(struct iovec) * ab->nr_entries); + if (addr) { + *addr = g_malloc0(sizeof(uint64_t) * ab->nr_entries); + } for (i = 0; i < ab->nr_entries; i++) { hwaddr len = ents[i].length; (*iov)[i].iov_len = ents[i].length; (*iov)[i].iov_base = cpu_physical_memory_map(ents[i].addr, &len, 1); + if (addr) { + (*addr)[i] = ents[i].addr; + } if (!(*iov)[i].iov_base || len != ents[i].length) { qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory for" " resource %d element %d\n", @@ -639,6 +652,10 @@ int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab, virtio_gpu_cleanup_mapping_iov(*iov, i); g_free(ents); *iov = NULL; + if (addr) { + g_free(*addr); + *addr = NULL; + } return -1; } } @@ -662,6 +679,8 @@ static void virtio_gpu_cleanup_mapping(struct virtio_gpu_simple_resource *res) virtio_gpu_cleanup_mapping_iov(res->iov, res->iov_cnt); res->iov = NULL; res->iov_cnt = 0; + g_free(res->addrs); + res->addrs = NULL; } static void @@ -683,7 +702,7 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g, return; } - ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->iov); + ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->addrs, &res->iov); if (ret != 0) { cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; return; @@ -929,11 +948,163 @@ const GraphicHwOps virtio_gpu_ops = { .gl_block = virtio_gpu_gl_block, }; +static const VMStateDescription vmstate_virtio_gpu_scanout = { + .name = "virtio-gpu-one-scanout", + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT32(resource_id, struct virtio_gpu_scanout), + VMSTATE_UINT32(width, struct virtio_gpu_scanout), + VMSTATE_UINT32(height, struct virtio_gpu_scanout), + VMSTATE_INT32(x, struct virtio_gpu_scanout), + VMSTATE_INT32(y, struct virtio_gpu_scanout), + VMSTATE_UINT32(cursor.resource_id, struct virtio_gpu_scanout), + VMSTATE_UINT32(cursor.hot_x, struct virtio_gpu_scanout), + VMSTATE_UINT32(cursor.hot_y, struct virtio_gpu_scanout), + VMSTATE_UINT32(cursor.pos.x, struct virtio_gpu_scanout), + VMSTATE_UINT32(cursor.pos.y, struct virtio_gpu_scanout), + VMSTATE_END_OF_LIST() + }, +}; + +static const VMStateDescription vmstate_virtio_gpu_scanouts = { + .name = "virtio-gpu-scanouts", + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_INT32(enable, struct VirtIOGPU), + VMSTATE_UINT32_EQUAL(conf.max_outputs, struct VirtIOGPU), + VMSTATE_STRUCT_VARRAY_UINT32(scanout, struct VirtIOGPU, + conf.max_outputs, 1, + vmstate_virtio_gpu_scanout, + struct virtio_gpu_scanout), + VMSTATE_END_OF_LIST() + }, +}; + static const VMStateDescription vmstate_virtio_gpu_unmigratable = { - .name = "virtio-gpu", + .name = "virtio-gpu-with-virgl", .unmigratable = 1, }; +static void virtio_gpu_save(QEMUFile *f, void *opaque) +{ + VirtIOGPU *g = opaque; + VirtIODevice *vdev = VIRTIO_DEVICE(g); + struct virtio_gpu_simple_resource *res; + int i; + + virtio_save(vdev, f); + + /* in 2d mode we should never find unprocessed commands here */ + assert(QTAILQ_EMPTY(&g->cmdq)); + + QTAILQ_FOREACH(res, &g->reslist, next) { + qemu_put_be32(f, res->resource_id); + qemu_put_be32(f, res->width); + qemu_put_be32(f, res->height); + qemu_put_be32(f, res->format); + qemu_put_be32(f, res->iov_cnt); + for (i = 0; i < res->iov_cnt; i++) { + qemu_put_be64(f, res->addrs[i]); + qemu_put_be32(f, res->iov[i].iov_len); + } + qemu_put_buffer(f, (void *)pixman_image_get_data(res->image), + pixman_image_get_stride(res->image) * res->height); + } + qemu_put_be32(f, 0); /* end of list */ + + vmstate_save_state(f, &vmstate_virtio_gpu_scanouts, g, NULL); +} + +static int virtio_gpu_load(QEMUFile *f, void *opaque, int version_id) +{ + VirtIOGPU *g = opaque; + VirtIODevice *vdev = VIRTIO_DEVICE(g); + struct virtio_gpu_simple_resource *res; + struct virtio_gpu_scanout *scanout; + uint32_t resource_id, pformat; + int i, ret; + + if (version_id != VIRTIO_GPU_VM_VERSION) { + return -EINVAL; + } + + ret = virtio_load(vdev, f, version_id); + if (ret) { + return ret; + } + + resource_id = qemu_get_be32(f); + while (resource_id != 0) { + res = g_new0(struct virtio_gpu_simple_resource, 1); + res->resource_id = resource_id; + res->width = qemu_get_be32(f); + res->height = qemu_get_be32(f); + res->format = qemu_get_be32(f); + res->iov_cnt = qemu_get_be32(f); + + /* allocate */ + pformat = get_pixman_format(res->format); + if (!pformat) { + return -EINVAL; + } + res->image = pixman_image_create_bits(pformat, + res->width, res->height, + NULL, 0); + if (!res->image) { + return -EINVAL; + } + + res->addrs = g_new(uint64_t, res->iov_cnt); + res->iov = g_new(struct iovec, res->iov_cnt); + + /* read data */ + for (i = 0; i < res->iov_cnt; i++) { + res->addrs[i] = qemu_get_be64(f); + res->iov[i].iov_len = qemu_get_be32(f); + } + qemu_get_buffer(f, (void *)pixman_image_get_data(res->image), + pixman_image_get_stride(res->image) * res->height); + + /* restore mapping */ + for (i = 0; i < res->iov_cnt; i++) { + hwaddr len = res->iov[i].iov_len; + res->iov[i].iov_base = + cpu_physical_memory_map(res->addrs[i], &len, 1); + if (!res->iov[i].iov_base || len != res->iov[i].iov_len) { + return -EINVAL; + } + } + + QTAILQ_INSERT_HEAD(&g->reslist, res, next); + + resource_id = qemu_get_be32(f); + } + + /* load & apply scanout state */ + vmstate_load_state(f, &vmstate_virtio_gpu_scanouts, g, 1); + for (i = 0; i < g->conf.max_outputs; i++) { + scanout = &g->scanout[i]; + if (!scanout->resource_id) { + continue; + } + res = virtio_gpu_find_resource(g, scanout->resource_id); + if (!res) { + return -EINVAL; + } + scanout->ds = qemu_create_displaysurface_pixman(res->image); + if (!scanout->ds) { + return -EINVAL; + } + + dpy_gfx_replace_surface(scanout->con, scanout->ds); + dpy_gfx_update(scanout->con, 0, 0, scanout->width, scanout->height); + update_cursor(g, &scanout->cursor); + res->scanout_bitmask |= (1 << i); + } + + return 0; +} + static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) { VirtIODevice *vdev = VIRTIO_DEVICE(qdev); @@ -991,7 +1162,12 @@ static void virtio_gpu_device_realize(DeviceState *qdev, Error **errp) } } - vmstate_register(qdev, -1, &vmstate_virtio_gpu_unmigratable, g); + if (virtio_gpu_virgl_enabled(g->conf)) { + vmstate_register(qdev, -1, &vmstate_virtio_gpu_unmigratable, g); + } else { + register_savevm(qdev, "virtio-gpu", -1, VIRTIO_GPU_VM_VERSION, + virtio_gpu_save, virtio_gpu_load, g); + } } static void virtio_gpu_instance_init(Object *obj) diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c index f49f8de..315b7fc 100644 --- a/hw/display/virtio-vga.c +++ b/hw/display/virtio-vga.c @@ -84,6 +84,17 @@ static const GraphicHwOps virtio_vga_ops = { .gl_block = virtio_vga_gl_block, }; +static const VMStateDescription vmstate_virtio_vga = { + .name = "virtio-vga", + .version_id = 2, + .minimum_version_id = 2, + .fields = (VMStateField[]) { + /* no pci stuff here, saving the virtio device will handle that */ + VMSTATE_STRUCT(vga, VirtIOVGA, 0, vmstate_vga_common, VGACommonState), + VMSTATE_END_OF_LIST() + } +}; + /* VGA device wrapper around PCI device around virtio GPU */ static void virtio_vga_realize(VirtIOPCIProxy *vpci_dev, Error **errp) { @@ -168,6 +179,7 @@ static void virtio_vga_class_init(ObjectClass *klass, void *data) set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories); dc->props = virtio_vga_properties; dc->reset = virtio_vga_reset; + dc->vmsd = &vmstate_virtio_vga; dc->hotpluggable = false; k->realize = virtio_vga_realize; diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 1602a13..89f4879 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -32,6 +32,7 @@ struct virtio_gpu_simple_resource { uint32_t width; uint32_t height; uint32_t format; + uint64_t *addrs; struct iovec *iov; unsigned int iov_cnt; uint32_t scanout_bitmask; @@ -46,6 +47,7 @@ struct virtio_gpu_scanout { int x, y; int invalidate; uint32_t resource_id; + struct virtio_gpu_update_cursor cursor; QEMUCursor *current_cursor; }; @@ -150,7 +152,7 @@ void virtio_gpu_get_display_info(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd); int virtio_gpu_create_mapping_iov(struct virtio_gpu_resource_attach_backing *ab, struct virtio_gpu_ctrl_command *cmd, - struct iovec **iov); + uint64_t **addr, struct iovec **iov); void virtio_gpu_cleanup_mapping_iov(struct iovec *iov, uint32_t count); void virtio_gpu_process_cmdq(VirtIOGPU *g); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support 2016-06-06 7:06 ` [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support Gerd Hoffmann @ 2016-06-14 13:26 ` Paolo Bonzini 0 siblings, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2016-06-14 13:26 UTC (permalink / raw) To: Gerd Hoffmann, qemu-devel; +Cc: Michael S. Tsirkin On 06/06/2016 09:06, Gerd Hoffmann wrote: > + while (resource_id != 0) { > + res = g_new0(struct virtio_gpu_simple_resource, 1); > + res->resource_id = resource_id; > + res->width = qemu_get_be32(f); > + res->height = qemu_get_be32(f); > + res->format = qemu_get_be32(f); > + res->iov_cnt = qemu_get_be32(f); > + > + /* allocate */ > + pformat = get_pixman_format(res->format); > + if (!pformat) { > + return -EINVAL; Coverity says, all the "return" statements in this loop leak res. Paolo > + } > + res->image = pixman_image_create_bits(pformat, > + res->width, res->height, > + NULL, 0); > + if (!res->image) { > + return -EINVAL; > + } > + > + res->addrs = g_new(uint64_t, res->iov_cnt); > + res->iov = g_new(struct iovec, res->iov_cnt); > + > + /* read data */ > + for (i = 0; i < res->iov_cnt; i++) { > + res->addrs[i] = qemu_get_be64(f); > + res->iov[i].iov_len = qemu_get_be32(f); > + } > + qemu_get_buffer(f, (void *)pixman_image_get_data(res->image), > + pixman_image_get_stride(res->image) * res->height); > + > + /* restore mapping */ > + for (i = 0; i < res->iov_cnt; i++) { > + hwaddr len = res->iov[i].iov_len; > + res->iov[i].iov_base = > + cpu_physical_memory_map(res->addrs[i], &len, 1); > + if (!res->iov[i].iov_base || len != res->iov[i].iov_len) { > + return -EINVAL; > + } > + } > + > + QTAILQ_INSERT_HEAD(&g->reslist, res, next); > + > + resource_id = qemu_get_be32(f); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Qemu-devel] [PULL v2 0/6] vga patch queue 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann ` (5 preceding siblings ...) 2016-06-06 7:06 ` [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support Gerd Hoffmann @ 2016-06-06 14:17 ` Peter Maydell 6 siblings, 0 replies; 10+ messages in thread From: Peter Maydell @ 2016-06-06 14:17 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: QEMU Developers On 6 June 2016 at 08:06, Gerd Hoffmann <kraxel@redhat.com> wrote: > Hi, > > vga patch queue brings some security fixes for vmware vga > and live migration support (in 2d mode) for virtio-gpu. > > v2: add qemu-stable to cc for the security fixes. > > please pull, > Gerd > > The following changes since commit 2c107d7684f9e3c4db4780d0756bbf35b06aec07: > > Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-06-02 14:26:57 +0100) > > are available in the git repository at: > > > git://git.kraxel.org/qemu tags/pull-vga-20160606-1 > > for you to fetch changes up to 0c244e50ee12311037efd507ee37df0e846e4a18: > > virtio-gpu: add live migration support (2016-06-06 09:04:34 +0200) > > ---------------------------------------------------------------- > virtio-gpu: scanout fix, live migration support > vmsvga: security fixes > > ---------------------------------------------------------------- Applied, thanks. -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Qemu-devel] [PULL 0/6] vga patch queue
@ 2016-06-03 7:34 Gerd Hoffmann
2016-06-03 7:34 ` [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length Gerd Hoffmann
0 siblings, 1 reply; 10+ messages in thread
From: Gerd Hoffmann @ 2016-06-03 7:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Hi,
vga patch queue brings some security fixes for vmware vga
and live migration support (in 2d mode) for virtio-gpu.
please pull,
Gerd
The following changes since commit 2c107d7684f9e3c4db4780d0756bbf35b06aec07:
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging (2016-06-02 14:26:57 +0100)
are available in the git repository at:
git://git.kraxel.org/qemu tags/pull-vga-20160603-1
for you to fetch changes up to 8d0cc96d0dc18931494437ffa68655c0e11dc8fe:
virtio-gpu: add live migration support (2016-06-03 09:06:37 +0200)
----------------------------------------------------------------
virtio-gpu: scanout fix, live migration support
vmsvga: security fixes
----------------------------------------------------------------
Gerd Hoffmann (6):
virtio-gpu: fix scanout rectangles
vmsvga: move fifo sanity checks to vmsvga_fifo_length
vmsvga: add more fifo checks
vmsvga: shadow fifo registers
vmsvga: don't process more than 1024 fifo commands at once
virtio-gpu: add live migration support
hw/display/virtio-gpu-3d.c | 2 +-
hw/display/virtio-gpu.c | 202 +++++++++++++++++++++++++++++++++++++++--
hw/display/virtio-vga.c | 12 +++
hw/display/vmware_vga.c | 78 ++++++++--------
include/hw/virtio/virtio-gpu.h | 4 +-
5 files changed, 252 insertions(+), 46 deletions(-)
^ permalink raw reply [flat|nested] 10+ messages in thread* [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length 2016-06-03 7:34 [Qemu-devel] [PULL " Gerd Hoffmann @ 2016-06-03 7:34 ` Gerd Hoffmann 0 siblings, 0 replies; 10+ messages in thread From: Gerd Hoffmann @ 2016-06-03 7:34 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann, P J P Sanity checks are applied when the fifo is enabled by the guest (SVGA_REG_CONFIG_DONE write). Which doesn't help much if the guest changes the fifo registers afterwards. Move the checks to vmsvga_fifo_length so they are done each time qemu is about to read from the fifo. Fixes: CVE-2016-4454 Cc: P J P <ppandit@redhat.com> Reported-by: 李强 <liqiang6-s@360.cn> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Message-id: 1464592161-18348-2-git-send-email-kraxel@redhat.com --- hw/display/vmware_vga.c | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 0c63fa8..63a7c05 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -555,6 +555,21 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s) if (!s->config || !s->enable) { return 0; } + + /* Check range and alignment. */ + if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { + return 0; + } + if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { + return 0; + } + if (CMD(max) > SVGA_FIFO_SIZE) { + return 0; + } + if (CMD(max) < CMD(min) + 10 * 1024) { + return 0; + } + num = CMD(next_cmd) - CMD(stop); if (num < 0) { num += CMD(max) - CMD(min); @@ -1005,19 +1020,6 @@ static void vmsvga_value_write(void *opaque, uint32_t address, uint32_t value) case SVGA_REG_CONFIG_DONE: if (value) { s->fifo = (uint32_t *) s->fifo_ptr; - /* Check range and alignment. */ - if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) { - break; - } - if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) { - break; - } - if (CMD(max) > SVGA_FIFO_SIZE) { - break; - } - if (CMD(max) < CMD(min) + 10 * 1024) { - break; - } vga_dirty_log_stop(&s->vga); } s->config = !!value; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-06-14 13:28 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-06-06 7:06 [Qemu-devel] [PULL v2 0/6] vga patch queue Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 1/6] virtio-gpu: fix scanout rectangles Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 3/6] vmsvga: add more fifo checks Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 4/6] vmsvga: shadow fifo registers Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 5/6] vmsvga: don't process more than 1024 fifo commands at once Gerd Hoffmann 2016-06-06 7:06 ` [Qemu-devel] [PULL 6/6] virtio-gpu: add live migration support Gerd Hoffmann 2016-06-14 13:26 ` Paolo Bonzini 2016-06-06 14:17 ` [Qemu-devel] [PULL v2 0/6] vga patch queue Peter Maydell -- strict thread matches above, loose matches on Subject: below -- 2016-06-03 7:34 [Qemu-devel] [PULL " Gerd Hoffmann 2016-06-03 7:34 ` [Qemu-devel] [PULL 2/6] vmsvga: move fifo sanity checks to vmsvga_fifo_length Gerd Hoffmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.