From aee386e957e8107f54aee7428b60333c7c7f2218 Mon Sep 17 00:00:00 2001 From: Eduardo Habkost <ehabkost@redhat.com> Date: Thu, 1 Jul 2010 16:45:07 -0300 Subject: [PATCH] qemu: fix unsafe ring handling Embargo date: 2010-08-12 It is almost the same patch that was previously submitted and ACKed. Please ACK. CVE: CVE-2010-0431 CVE Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=568809 5.5.z BZ: https://bugzilla.redhat.com/show_bug.cgi?id=568816 5.6 BZ: https://bugzilla.redhat.com/show_bug.cgi?id=568817 Acked-by: Gerd Hoffmann <kraxel@redhat.com> Acked-by: Yonit Halperin <yhalperi@redhat.com> Acked-by: Uri Lublin <uril@redhat.com> -------------------- >From 1690d9b5b4f72007c09ea6ee069c503b4eea53f8 Mon Sep 17 00:00:00 2001 From: Izik Eidus <ieidus@redhat.com> Date: Tue, 4 May 2010 10:36:24 +0300 Signed-off-by: Izik Eidus <ieidus@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- qemu/hw/qxl.c | 38 +++++++++++++++++++++++++++----------- qemu/hw/ring.h | 23 +++++++++++++++++++++-- 2 files changed, 48 insertions(+), 13 deletions(-) diff --git a/qemu/hw/qxl.c b/qemu/hw/qxl.c index 8778a7d..bcba720 100644 --- a/qemu/hw/qxl.c +++ b/qemu/hw/qxl.c @@ -366,12 +366,14 @@ static QXLCommandRing *qxl_active_ring(PCIQXLDevice *d) static int _qxl_get_command(PCIQXLDevice *d, QXLCommand *cmd) { QXLCommandRing *ring = qxl_active_ring(d); + QXLCommand *tmp_cmd; int notify; if (RING_IS_EMPTY(ring)) { return FALSE; } - *cmd = *RING_CONS_ITEM(ring); + RING_CONS_ITEM(ring, tmp_cmd); + *cmd = *tmp_cmd; RING_POP(ring, notify); if (d->state.mode != QXL_MODE_VGA && notify) { qxl_send_events(d, QXL_INTERRUPT_DISPLAY); @@ -388,6 +390,7 @@ static int _qxl_has_command(PCIQXLDevice *d) static int _qxl_get_cursor_command(PCIQXLDevice *d, QXLCommand *cmd) { QXLCursorRing *ring; + QXLCommand *tmp_cmd; int notify; if (d->state.mode == QXL_MODE_VGA) { @@ -399,7 +402,8 @@ static int _qxl_get_cursor_command(PCIQXLDevice *d, QXLCommand *cmd) if (RING_IS_EMPTY(ring)) { return 0; } - *cmd = *RING_CONS_ITEM(ring); + RING_CONS_ITEM(ring,tmp_cmd); + *cmd = *tmp_cmd; RING_POP(ring, notify); if (d->state.mode != QXL_MODE_VGA && notify) { qxl_send_events(d, QXL_INTERRUPT_CURSOR); @@ -437,6 +441,7 @@ static inline void qxl_push_free_res(PCIQXLDevice *d) { QXLState *state = &d->state; QXLReleaseRing *ring = &state->ram->release_ring; + UINT64 *item; ASSERT(state->mode != QXL_MODE_VGA); if (RING_IS_EMPTY(ring) || (state->num_free_res == QXL_FREE_BUNCH_SIZE && @@ -447,7 +452,8 @@ static inline void qxl_push_free_res(PCIQXLDevice *d) if (notify) { qxl_send_events(d, QXL_INTERRUPT_DISPLAY); } - *RING_PROD_ITEM(ring) = 0; + RING_PROD_ITEM(ring, item); + *item = 0; state->num_free_res = 0; state->last_release = NULL; } @@ -465,7 +471,7 @@ static void _qxl_release_resource(PCIQXLDevice *d, QXLReleaseInfo *release_info) return; } ring = &state->ram->release_ring; - item = RING_PROD_ITEM(ring); + RING_PROD_ITEM(ring, item); if (*item == 0) { release_info->next = 0; *item = id; @@ -541,7 +547,7 @@ static void qxl_detach(PCIQXLDevice *d) QXLCommand *cmd; int notify; - cmd = RING_CONS_ITEM(ring); + RING_CONS_ITEM(ring, cmd); RING_POP(ring, notify); ASSERT(cmd->type == QXL_CMD_DRAW); drawable = (QXLDrawable *)cmd->data; @@ -639,6 +645,7 @@ static void qxl_reset_state(PCIQXLDevice *d) { QXLRam *ram = d->state.ram; QXLRom *rom = d->state.rom; + UINT64 *item; ASSERT(RING_IS_EMPTY(&ram->cmd_ring)); ASSERT(RING_IS_EMPTY(&ram->cursor_ring)); @@ -652,7 +659,8 @@ static void qxl_reset_state(PCIQXLDevice *d) RING_INIT(&ram->cursor_ring); RING_INIT(&d->state.vga_ring); RING_INIT(&ram->release_ring); - *RING_PROD_ITEM(&ram->release_ring) = 0; + RING_PROD_ITEM(&ram->release_ring, item); + *item = 0; d->state.num_free_res = 0; d->state.last_release = NULL; } @@ -763,7 +771,7 @@ static int vdi_port_interface_write(VDIPortInterface *port, VDObjectRef plug, break; } - packet = RING_PROD_ITEM(ring); + RING_PROD_ITEM(ring, packet); packet->gen = d->ram->generation; packet->size = MIN(len, sizeof(packet->data)); memcpy(packet->data, buf, packet->size); @@ -789,6 +797,7 @@ static int vdi_port_interface_read(VDIPortInterface *port, VDObjectRef plug, PCIVDIPortDevice *d = container_of(port, PCIVDIPortDevice, interface); VDIPortRing *ring = &d->ram->input; uint32_t gen = d->ram->generation; + VDIPortPacket *packet; int do_notify = FALSE; int actual_read = 0; @@ -796,9 +805,14 @@ static int vdi_port_interface_read(VDIPortInterface *port, VDObjectRef plug, return 0; } - while (!RING_IS_EMPTY(ring) && RING_CONS_ITEM(ring)->gen != gen) { + while (!RING_IS_EMPTY(ring)) { int notify; + RING_CONS_ITEM(ring, packet); + if (packet->gen == gen) { + break; + } + RING_POP(ring, notify); do_notify = do_notify || notify; } @@ -813,7 +827,7 @@ static int vdi_port_interface_read(VDIPortInterface *port, VDObjectRef plug, break; } - packet = RING_CONS_ITEM(ring); + RING_CONS_ITEM(ring, packet); if (packet->size > sizeof(packet->data)) { vdi_port_set_dirty(d, ring, sizeof(*ring) - sizeof(ring->items)); printf("%s: bad packet size\n", __FUNCTION__); @@ -1110,6 +1124,7 @@ static void init_qxl_ram(QXLState *s, uint8_t *buf, uint32_t actual_ram_size) { uint32_t draw_area_size; uint32_t ram_header_size; + UINT64 *item; s->ram_start = buf; @@ -1123,7 +1138,8 @@ static void init_qxl_ram(QXLState *s, uint8_t *buf, uint32_t actual_ram_size) RING_INIT(&s->ram->cmd_ring); RING_INIT(&s->ram->cursor_ring); RING_INIT(&s->ram->release_ring); - *RING_PROD_ITEM(&s->ram->release_ring) = 0; + RING_PROD_ITEM(&s->ram->release_ring, item); + *item = 0; s->shadow_rom.draw_area_offset = s->shadow_rom.ram_header_offset - draw_area_size; s->shadow_rom.pages_offset = 0; @@ -1224,7 +1240,7 @@ static void qxl_vga_update_one(PCIQXLDevice *d) dirty_rect->left * 4); image->bitmap.palette = 0; - cmd = RING_PROD_ITEM(ring); + RING_PROD_ITEM(ring, cmd); cmd->type = QXL_CMD_DRAW; cmd->data = (PHYSICAL)drawable; RING_PUSH(ring, notify); diff --git a/qemu/hw/ring.h b/qemu/hw/ring.h index 9653f01..14964fc 100644 --- a/qemu/hw/ring.h +++ b/qemu/hw/ring.h @@ -70,7 +70,17 @@ typedef struct ATTR_PACKED name { \ #define RING_IS_FULL(r) (((r)->prod - (r)->cons) == (r)->num_items) -#define RING_PROD_ITEM(r) (&(r)->items[(r)->prod & RING_INDEX_MASK(r)].el) + +#define RING_PROD_ITEM(r, ret) { \ + typeof(r) start = r; \ + typeof(r) end = r + 1; \ + UINT32 prod = (r)->prod & RING_INDEX_MASK(r); \ + typeof(&(r)->items[prod]) m_item = &(r)->items[prod]; \ + if (!((uint8_t*)m_item >= (uint8_t*)(start) && (uint8_t*)(m_item + 1) <= (uint8_t*)(end))) { \ + abort(); \ + } \ + ret = &m_item->el; \ +} #define RING_PROD_WAIT(r, wait) \ if (((wait) = RING_IS_FULL(r))) { \ @@ -85,7 +95,16 @@ typedef struct ATTR_PACKED name { \ (notify) = (r)->prod == (r)->notify_on_prod; -#define RING_CONS_ITEM(r) (&(r)->items[(r)->cons & RING_INDEX_MASK(r)].el) +#define RING_CONS_ITEM(r, ret) { \ + typeof(r) start = r; \ + typeof(r) end = r + 1; \ + UINT32 cons = (r)->cons & RING_INDEX_MASK(r); \ + typeof(&(r)->items[cons]) m_item = &(r)->items[cons]; \ + if (!((uint8_t*)m_item >= (uint8_t*)(start) && (uint8_t*)(m_item + 1) <= (uint8_t*)(end))) { \ + abort(); \ + } \ + ret = &m_item->el; \ +} #define RING_CONS_WAIT(r, wait) \ if (((wait) = RING_IS_EMPTY(r))) { \ -- 1.7.0.3