145 lines
5.3 KiB
Diff
145 lines
5.3 KiB
Diff
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
||
|
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é <berrange@redhat.com>
|
||
|
Message-Id: <20190412121626.19829-6-berrange@redhat.com>
|
||
|
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
|
||
|
(cherry picked from commit 94932c95c10400acd286fd768a6b411e7ebbec8f)
|
||
|
Signed-off-by: Bruce Rogers <brogers@suse.com>
|
||
|
---
|
||
|
hw/display/qxl.c | 55 +++++++++++++++++++++---------------------------
|
||
|
1 file changed, 24 insertions(+), 31 deletions(-)
|
||
|
|
||
|
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
|
||
|
index c8ce5781e0..5c38e6e906 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.group_id == MEMSLOT_GROUP_HOST) {
|
||
|
/* host group -> vga mode update request */
|
||
|
@@ -792,16 +783,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 */
|