From d52680fc932efb8a2f334cc6993e705ed1e31e99 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Thu, 25 Apr 2019 12:05:34 +0530 Subject: qxl: check release info object When releasing spice resources in release_resource() routine, if release info object 'ext.info' is null, it leads to null pointer dereference. Add check to avoid it. Reported-by: Bugs SysSec Signed-off-by: Prasad J Pandit Message-id: 20190425063534.32747-1-ppandit@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'hw') diff --git a/hw/display/qxl.c b/hw/display/qxl.c index c8ce5781e0..632923add2 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -777,6 +777,9 @@ static void interface_release_resource(QXLInstance *sin, QXLReleaseRing *ring; uint64_t *item, id; + if (!ext.info) { + return; + } if (ext.group_id == MEMSLOT_GROUP_HOST) { /* host group -> vga mode update request */ QXLCommandExt *cmdext = (void *)(intptr_t)(ext.info->id); -- cgit v1.2.3 From 295854686eaff0edb7e27b14c8b47ca18a9f31b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Sat, 4 May 2019 14:16:50 +0200 Subject: hw/display/cirrus_vga: Update the documentation URL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The documentation URL is not working, but is backed up by the Wayback Machine on the Internet Archive. Replace the outdated link by a captured one. Add another link to the VGADOC4b.ZIP archive content. Signed-off-by: Philippe Mathieu-Daudé Message-id: 20190504121650.12651-1-philmd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/cirrus_vga.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index a0e71469f4..a04440b374 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -23,8 +23,13 @@ * THE SOFTWARE. */ /* - * Reference: Finn Thogersons' VGADOC4b - * available at http://home.worldonline.dk/~finth/ + * Reference: Finn Thogersons' VGADOC4b: + * + * http://web.archive.org/web/20021019054927/http://home.worldonline.dk/finth/ + * + * VGADOC4b.ZIP content available at: + * + * https://pdos.csail.mit.edu/6.828/2005/readings/hardware/vgadoc */ #include "qemu/osdep.h" #include "qemu/units.h" -- cgit v1.2.3 From fad691db49454f3cd64283ccc6cdf554a68c727f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Mon, 6 May 2019 00:56:40 +0200 Subject: hw/display/cirrus_vga: Remove unused include MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit ce3cf70edaaf split the ISA device out of the PCI one, but forgot to remove the "hw/loader.h" header inclusion (the ISA device calls rom_add_vga()). Remove the now unused include. Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Message-id: 20190505225640.4592-1-philmd@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/cirrus_vga.c | 1 - 1 file changed, 1 deletion(-) (limited to 'hw') diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c index a04440b374..76c052c702 100644 --- a/hw/display/cirrus_vga.c +++ b/hw/display/cirrus_vga.c @@ -38,7 +38,6 @@ #include "hw/hw.h" #include "hw/pci/pci.h" #include "ui/pixel_ops.h" -#include "hw/loader.h" #include "cirrus_vga_internal.h" /* -- cgit v1.2.3 From 94932c95c10400acd286fd768a6b411e7ebbec8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 12 Apr 2019 13:16:26 +0100 Subject: qxl: avoid unaligned pointer reads/writes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SPICE_RING_PROD_ITEM() macro is initializing a local 'uint64_t *' variable to point to the 'el' field inside the QXLReleaseRing struct. This uint64_t field is not guaranteed aligned as the struct is packed. Code should not take the address of fields within a packed struct. Changing the SPICE_RING_PROD_ITEM() macro to avoid taking the address of the field is impractical. It is clearer to just remove the macro and inline its functionality in the three call sites that need it. Signed-off-by: Daniel P. Berrangé Message-Id: <20190412121626.19829-6-berrange@redhat.com> Signed-off-by: Gerd Hoffmann --- hw/display/qxl.c | 55 ++++++++++++++++++++++++------------------------------- 1 file changed, 24 insertions(+), 31 deletions(-) (limited to 'hw') diff --git a/hw/display/qxl.c b/hw/display/qxl.c index 632923add2..3880a7410b 100644 --- a/hw/display/qxl.c +++ b/hw/display/qxl.c @@ -33,24 +33,6 @@ #include "qxl.h" -/* - * NOTE: SPICE_RING_PROD_ITEM accesses memory on the pci bar and as - * such can be changed by the guest, so to avoid a guest trigerrable - * abort we just qxl_set_guest_bug and set the return to NULL. Still - * it may happen as a result of emulator bug as well. - */ -#undef SPICE_RING_PROD_ITEM -#define SPICE_RING_PROD_ITEM(qxl, r, ret) { \ - uint32_t prod = (r)->prod & SPICE_RING_INDEX_MASK(r); \ - if (prod >= ARRAY_SIZE((r)->items)) { \ - qxl_set_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " \ - "%u >= %zu", prod, ARRAY_SIZE((r)->items)); \ - ret = NULL; \ - } else { \ - ret = &(r)->items[prod].el; \ - } \ - } - #undef SPICE_RING_CONS_ITEM #define SPICE_RING_CONS_ITEM(qxl, r, ret) { \ uint32_t cons = (r)->cons & SPICE_RING_INDEX_MASK(r); \ @@ -414,7 +396,8 @@ static void init_qxl_rom(PCIQXLDevice *d) static void init_qxl_ram(PCIQXLDevice *d) { uint8_t *buf; - uint64_t *item; + uint32_t prod; + QXLReleaseRing *ring; buf = d->vga.vram_ptr; d->ram = (QXLRam *)(buf + le32_to_cpu(d->shadow_rom.ram_header_offset)); @@ -426,9 +409,12 @@ static void init_qxl_ram(PCIQXLDevice *d) SPICE_RING_INIT(&d->ram->cmd_ring); SPICE_RING_INIT(&d->ram->cursor_ring); SPICE_RING_INIT(&d->ram->release_ring); - SPICE_RING_PROD_ITEM(d, &d->ram->release_ring, item); - assert(item); - *item = 0; + + ring = &d->ram->release_ring; + prod = ring->prod & SPICE_RING_INDEX_MASK(ring); + assert(prod < ARRAY_SIZE(ring->items)); + ring->items[prod].el = 0; + qxl_ring_set_dirty(d); } @@ -732,7 +718,7 @@ static int interface_req_cmd_notification(QXLInstance *sin) static inline void qxl_push_free_res(PCIQXLDevice *d, int flush) { QXLReleaseRing *ring = &d->ram->release_ring; - uint64_t *item; + uint32_t prod; int notify; #define QXL_FREE_BUNCH_SIZE 32 @@ -759,11 +745,15 @@ static inline void qxl_push_free_res(PCIQXLDevice *d, int flush) if (notify) { qxl_send_events(d, QXL_INTERRUPT_DISPLAY); } - SPICE_RING_PROD_ITEM(d, ring, item); - if (!item) { + + ring = &d->ram->release_ring; + prod = ring->prod & SPICE_RING_INDEX_MASK(ring); + if (prod >= ARRAY_SIZE(ring->items)) { + qxl_set_guest_bug(d, "SPICE_RING_PROD_ITEM indices mismatch " + "%u >= %zu", prod, ARRAY_SIZE(ring->items)); return; } - *item = 0; + ring->items[prod].el = 0; d->num_free_res = 0; d->last_release = NULL; qxl_ring_set_dirty(d); @@ -775,7 +765,8 @@ static void interface_release_resource(QXLInstance *sin, { PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl); QXLReleaseRing *ring; - uint64_t *item, id; + uint32_t prod; + uint64_t id; if (!ext.info) { return; @@ -795,16 +786,18 @@ static void interface_release_resource(QXLInstance *sin, * pci bar 0, $command.release_info */ ring = &qxl->ram->release_ring; - SPICE_RING_PROD_ITEM(qxl, ring, item); - if (!item) { + prod = ring->prod & SPICE_RING_INDEX_MASK(ring); + if (prod >= ARRAY_SIZE(ring->items)) { + qxl_set_guest_bug(qxl, "SPICE_RING_PROD_ITEM indices mismatch " + "%u >= %zu", prod, ARRAY_SIZE(ring->items)); return; } - if (*item == 0) { + if (ring->items[prod].el == 0) { /* stick head into the ring */ id = ext.info->id; ext.info->next = 0; qxl_ram_set_dirty(qxl, &ext.info->next); - *item = id; + ring->items[prod].el = id; qxl_ring_set_dirty(qxl); } else { /* append item to the list */ -- cgit v1.2.3 From 349ebdd76d3a932204f5831950a2af413c29c477 Mon Sep 17 00:00:00 2001 From: BALATON Zoltan Date: Tue, 9 Apr 2019 12:56:18 +0200 Subject: ati-vga: Fix check for blt outside vram Fix the check preventing calling pixman functions that would access memory outside allocated vram. The r128 X driver sometimes seem to try blits that span outside vram, this check prevents crashing QEMU in that case. (The r128 X driver may have problems even on real hardware so I'm not sure if it's a client bug or emulation problem but at least QEMU should survive.) Signed-off-by: BALATON Zoltan Tested-by: Andrew Randrianasulu Message-Id: <20190409110732.5C5FF7465DB@zero.eik.bme.hu> Signed-off-by: Gerd Hoffmann --- hw/display/ati_2d.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'hw') diff --git a/hw/display/ati_2d.c b/hw/display/ati_2d.c index bc98ba6eeb..fe3ae14864 100644 --- a/hw/display/ati_2d.c +++ b/hw/display/ati_2d.c @@ -79,10 +79,10 @@ void ati_2d_blt(ATIVGAState *s) s->regs.dst_width, s->regs.dst_height); end = s->vga.vram_ptr + s->vga.vram_size; if (src_bits >= end || dst_bits >= end || - src_bits + (s->regs.src_y + s->regs.dst_height) * src_stride + - s->regs.src_x >= end || - dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride + - s->regs.dst_x >= end) { + src_bits + s->regs.src_x + (s->regs.src_y + s->regs.dst_height) * + src_stride * sizeof(uint32_t) >= end || + dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) * + dst_stride * sizeof(uint32_t) >= end) { qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n"); return; } @@ -140,8 +140,8 @@ void ati_2d_blt(ATIVGAState *s) filler); end = s->vga.vram_ptr + s->vga.vram_size; if (dst_bits >= end || - dst_bits + (s->regs.dst_y + s->regs.dst_height) * dst_stride + - s->regs.dst_x >= end) { + dst_bits + s->regs.dst_x + (s->regs.dst_y + s->regs.dst_height) * + dst_stride * sizeof(uint32_t) >= end) { qemu_log_mask(LOG_UNIMP, "blt outside vram not implemented\n"); return; } -- cgit v1.2.3 From 6306cae275c7091aa4e785809d956b475bfedab4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 25 Mar 2019 16:59:23 +0100 Subject: i2c-ddc: move it to hw/display MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move it together with the other EDID code. hw/i2c should only include the core and the adapters, not the slaves. Signed-off-by: Paolo Bonzini Reviewed-by: Philippe Mathieu-Daudé Message-id: 20190325155923.30987-1-pbonzini@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/Kconfig | 5 ++ hw/display/Makefile.objs | 1 + hw/display/i2c-ddc.c | 127 +++++++++++++++++++++++++++++++++++++++++++++++ hw/display/sii9022.c | 2 +- hw/display/sm501.c | 2 +- hw/i2c/Kconfig | 5 -- hw/i2c/Makefile.objs | 1 - hw/i2c/i2c-ddc.c | 127 ----------------------------------------------- 8 files changed, 135 insertions(+), 135 deletions(-) create mode 100644 hw/display/i2c-ddc.c delete mode 100644 hw/i2c/i2c-ddc.c (limited to 'hw') diff --git a/hw/display/Kconfig b/hw/display/Kconfig index 72be57a403..c236cd2d0a 100644 --- a/hw/display/Kconfig +++ b/hw/display/Kconfig @@ -1,3 +1,8 @@ +config DDC + bool + depends on I2C + select EDID + config EDID bool diff --git a/hw/display/Makefile.objs b/hw/display/Makefile.objs index dbd453ab1b..650031f725 100644 --- a/hw/display/Makefile.objs +++ b/hw/display/Makefile.objs @@ -1,3 +1,4 @@ +common-obj-$(CONFIG_DDC) += i2c-ddc.o common-obj-$(CONFIG_EDID) += edid-generate.o edid-region.o common-obj-$(CONFIG_FW_CFG_DMA) += ramfb.o diff --git a/hw/display/i2c-ddc.c b/hw/display/i2c-ddc.c new file mode 100644 index 0000000000..9fe5403a92 --- /dev/null +++ b/hw/display/i2c-ddc.c @@ -0,0 +1,127 @@ +/* A simple I2C slave for returning monitor EDID data via DDC. + * + * Copyright (c) 2011 Linaro Limited + * Written by Peter Maydell + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, see . + */ + +#include "qemu/osdep.h" +#include "qemu-common.h" +#include "qemu/log.h" +#include "hw/i2c/i2c.h" +#include "hw/display/i2c-ddc.h" + +#ifndef DEBUG_I2CDDC +#define DEBUG_I2CDDC 0 +#endif + +#define DPRINTF(fmt, ...) do { \ + if (DEBUG_I2CDDC) { \ + qemu_log("i2c-ddc: " fmt , ## __VA_ARGS__); \ + } \ +} while (0) + +static void i2c_ddc_reset(DeviceState *ds) +{ + I2CDDCState *s = I2CDDC(ds); + + s->firstbyte = false; + s->reg = 0; +} + +static int i2c_ddc_event(I2CSlave *i2c, enum i2c_event event) +{ + I2CDDCState *s = I2CDDC(i2c); + + if (event == I2C_START_SEND) { + s->firstbyte = true; + } + + return 0; +} + +static uint8_t i2c_ddc_rx(I2CSlave *i2c) +{ + I2CDDCState *s = I2CDDC(i2c); + + int value; + value = s->edid_blob[s->reg % sizeof(s->edid_blob)]; + s->reg++; + return value; +} + +static int i2c_ddc_tx(I2CSlave *i2c, uint8_t data) +{ + I2CDDCState *s = I2CDDC(i2c); + if (s->firstbyte) { + s->reg = data; + s->firstbyte = false; + DPRINTF("[EDID] Written new pointer: %u\n", data); + return 0; + } + + /* Ignore all writes */ + s->reg++; + return 0; +} + +static void i2c_ddc_init(Object *obj) +{ + I2CDDCState *s = I2CDDC(obj); + + qemu_edid_generate(s->edid_blob, sizeof(s->edid_blob), &s->edid_info); +} + +static const VMStateDescription vmstate_i2c_ddc = { + .name = TYPE_I2CDDC, + .version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_BOOL(firstbyte, I2CDDCState), + VMSTATE_UINT8(reg, I2CDDCState), + VMSTATE_END_OF_LIST() + } +}; + +static Property i2c_ddc_properties[] = { + DEFINE_EDID_PROPERTIES(I2CDDCState, edid_info), + DEFINE_PROP_END_OF_LIST(), +}; + +static void i2c_ddc_class_init(ObjectClass *oc, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(oc); + I2CSlaveClass *isc = I2C_SLAVE_CLASS(oc); + + dc->reset = i2c_ddc_reset; + dc->vmsd = &vmstate_i2c_ddc; + dc->props = i2c_ddc_properties; + isc->event = i2c_ddc_event; + isc->recv = i2c_ddc_rx; + isc->send = i2c_ddc_tx; +} + +static TypeInfo i2c_ddc_info = { + .name = TYPE_I2CDDC, + .parent = TYPE_I2C_SLAVE, + .instance_size = sizeof(I2CDDCState), + .instance_init = i2c_ddc_init, + .class_init = i2c_ddc_class_init +}; + +static void ddc_register_devices(void) +{ + type_register_static(&i2c_ddc_info); +} + +type_init(ddc_register_devices); diff --git a/hw/display/sii9022.c b/hw/display/sii9022.c index 9994385c35..9c36e4c17e 100644 --- a/hw/display/sii9022.c +++ b/hw/display/sii9022.c @@ -16,7 +16,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "hw/i2c/i2c.h" -#include "hw/i2c/i2c-ddc.h" +#include "hw/display/i2c-ddc.h" #include "trace.h" #define SII9022_SYS_CTRL_DATA 0x1a diff --git a/hw/display/sm501.c b/hw/display/sm501.c index 2122291308..1e2709b2d0 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -35,7 +35,7 @@ #include "hw/sysbus.h" #include "hw/pci/pci.h" #include "hw/i2c/i2c.h" -#include "hw/i2c/i2c-ddc.h" +#include "hw/display/i2c-ddc.h" #include "qemu/range.h" #include "ui/pixel_ops.h" #include "qemu/bswap.h" diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig index 820b24de5b..78a2008e3a 100644 --- a/hw/i2c/Kconfig +++ b/hw/i2c/Kconfig @@ -5,11 +5,6 @@ config SMBUS_EEPROM bool depends on I2C -config DDC - bool - depends on I2C - select EDID - config VERSATILE_I2C bool select I2C diff --git a/hw/i2c/Makefile.objs b/hw/i2c/Makefile.objs index 5f76b6a990..d7073a401f 100644 --- a/hw/i2c/Makefile.objs +++ b/hw/i2c/Makefile.objs @@ -1,6 +1,5 @@ common-obj-$(CONFIG_I2C) += core.o smbus_slave.o smbus_master.o common-obj-$(CONFIG_SMBUS_EEPROM) += smbus_eeprom.o -common-obj-$(CONFIG_DDC) += i2c-ddc.o common-obj-$(CONFIG_VERSATILE_I2C) += versatile_i2c.o common-obj-$(CONFIG_ACPI_X86_ICH) += smbus_ich9.o common-obj-$(CONFIG_ACPI_SMBUS) += pm_smbus.o diff --git a/hw/i2c/i2c-ddc.c b/hw/i2c/i2c-ddc.c deleted file mode 100644 index 7aa8727771..0000000000 --- a/hw/i2c/i2c-ddc.c +++ /dev/null @@ -1,127 +0,0 @@ -/* A simple I2C slave for returning monitor EDID data via DDC. - * - * Copyright (c) 2011 Linaro Limited - * Written by Peter Maydell - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, see . - */ - -#include "qemu/osdep.h" -#include "qemu-common.h" -#include "qemu/log.h" -#include "hw/i2c/i2c.h" -#include "hw/i2c/i2c-ddc.h" - -#ifndef DEBUG_I2CDDC -#define DEBUG_I2CDDC 0 -#endif - -#define DPRINTF(fmt, ...) do { \ - if (DEBUG_I2CDDC) { \ - qemu_log("i2c-ddc: " fmt , ## __VA_ARGS__); \ - } \ -} while (0) - -static void i2c_ddc_reset(DeviceState *ds) -{ - I2CDDCState *s = I2CDDC(ds); - - s->firstbyte = false; - s->reg = 0; -} - -static int i2c_ddc_event(I2CSlave *i2c, enum i2c_event event) -{ - I2CDDCState *s = I2CDDC(i2c); - - if (event == I2C_START_SEND) { - s->firstbyte = true; - } - - return 0; -} - -static uint8_t i2c_ddc_rx(I2CSlave *i2c) -{ - I2CDDCState *s = I2CDDC(i2c); - - int value; - value = s->edid_blob[s->reg % sizeof(s->edid_blob)]; - s->reg++; - return value; -} - -static int i2c_ddc_tx(I2CSlave *i2c, uint8_t data) -{ - I2CDDCState *s = I2CDDC(i2c); - if (s->firstbyte) { - s->reg = data; - s->firstbyte = false; - DPRINTF("[EDID] Written new pointer: %u\n", data); - return 0; - } - - /* Ignore all writes */ - s->reg++; - return 0; -} - -static void i2c_ddc_init(Object *obj) -{ - I2CDDCState *s = I2CDDC(obj); - - qemu_edid_generate(s->edid_blob, sizeof(s->edid_blob), &s->edid_info); -} - -static const VMStateDescription vmstate_i2c_ddc = { - .name = TYPE_I2CDDC, - .version_id = 1, - .fields = (VMStateField[]) { - VMSTATE_BOOL(firstbyte, I2CDDCState), - VMSTATE_UINT8(reg, I2CDDCState), - VMSTATE_END_OF_LIST() - } -}; - -static Property i2c_ddc_properties[] = { - DEFINE_EDID_PROPERTIES(I2CDDCState, edid_info), - DEFINE_PROP_END_OF_LIST(), -}; - -static void i2c_ddc_class_init(ObjectClass *oc, void *data) -{ - DeviceClass *dc = DEVICE_CLASS(oc); - I2CSlaveClass *isc = I2C_SLAVE_CLASS(oc); - - dc->reset = i2c_ddc_reset; - dc->vmsd = &vmstate_i2c_ddc; - dc->props = i2c_ddc_properties; - isc->event = i2c_ddc_event; - isc->recv = i2c_ddc_rx; - isc->send = i2c_ddc_tx; -} - -static TypeInfo i2c_ddc_info = { - .name = TYPE_I2CDDC, - .parent = TYPE_I2C_SLAVE, - .instance_size = sizeof(I2CDDCState), - .instance_init = i2c_ddc_init, - .class_init = i2c_ddc_class_init -}; - -static void ddc_register_devices(void) -{ - type_register_static(&i2c_ddc_info); -} - -type_init(ddc_register_devices); -- cgit v1.2.3