Hi,
On 01/17/2013 09:26 AM, Uri Lublin wrote:
Resolves: rhbz#883578
Call qxl_allocate_monitors_config upon memory-remap such
that qxl->monitors_config points to the start of
monitors_config segment in qxl RAM memory.
Currently after memory remap, it's possible that monitors_config
memory and video-memory (or graphics) overlap, which means
that one may overwrite another.
Specifically in the bug above, monitors_config value are being
overwritten by video pages, and on the destination bad values
are read which cause problems on the server and client.
Can you please explain the path that leads to this overwriting?
I see that in qxl_map_memory qxl_allocate_monitors_config is already called.
It may be a good idea to add some protection on the server side,
e.g. calcluate checksum, compare values against modes, or limit
->count and ->max_allowed and ignore bad monitors_config values
Also do not memset-0 monitors-config upon allocation (remapping)
to not overwrite likely good configuration (in case it is
being read by the host, e.g. upon migration).
I'm not sure that the code in qemu-qxl should even re-read the monitors
configuration during pre-save because it was already updated on the
UPDATE_MONITORS_CONFIG io call.
Regards,
Yonit.
---
src/qxl_driver.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/src/qxl_driver.c b/src/qxl_driver.c
index 05c357b..e1893dc 100644
--- a/src/qxl_driver.c
+++ b/src/qxl_driver.c
@@ -134,6 +134,10 @@ const OptionInfoRec DefaultOptions[] =
{ -1, NULL, OPTV_NONE, {0}, FALSE }
};
+
+static void qxl_update_monitors_config (qxl_screen_t *qxl);
+
+
static const OptionInfoRec *
qxl_available_options (int chipid, int busid)
{
@@ -318,15 +322,8 @@ qxl_io_monitors_config_async (qxl_screen_t *qxl)
static void
qxl_allocate_monitors_config (qxl_screen_t *qxl)
{
- int size = sizeof (QXLMonitorsConfig) + sizeof (QXLHead) * MAX_MONITORS_NUM;
-
- if (qxl->monitors_config)
- return;
-
qxl->monitors_config = (QXLMonitorsConfig *)(void *)
((unsigned long)qxl->ram + qxl->rom->ram_header_offset - qxl->monitors_config_size);
-
- memset (qxl->monitors_config, 0, size);
}
static uint64_t
@@ -845,6 +842,8 @@ qxl_reset_and_create_mem_slots (qxl_screen_t *qxl)
(uint64_t)(uintptr_t)qxl->vram,
(uint64_t)(uintptr_t)qxl->vram + (uint64_t)qxl->vram_size);
#endif
+
+ qxl_allocate_monitors_config(qxl);
}
static void
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel