Bug 537888 - fix unsafe device data handling
Summary: fix unsafe device data handling
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: kvm
Version: 5.5
Hardware: All
OS: Linux
medium
medium
Target Milestone: rc
: 5.5
Assignee: Izik Eidus
QA Contact: Virtualization Bugs
URL:
Whiteboard:
: 568726 568727 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-11-16 17:16 UTC by Izik Eidus
Modified: 2010-04-29 20:01 UTC (History)
8 users (show)

Fixed In Version: kvm-83-149.el5
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-30 07:55:03 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2010:0271 0 normal SHIPPED_LIVE Important: kvm security, bug fix and enhancement update 2010-03-29 13:19:48 UTC

Description Izik Eidus 2009-11-16 17:16:08 UTC
Description of problem:

Guest driver can change rom qxl device data that might result in memory corruption.

Comment 5 lihuang 2010-01-26 18:28:32 UTC
Hi. Izik
could you please tell me how to test this bug ?

Comment 6 Shuxi Shang 2010-02-03 05:51:38 UTC
Hello, Izik:
   How to reproduce it?

Comment 7 Keqin Hong 2010-03-01 05:39:57 UTC
>From: Izik Eidus <ieidus redhat com>
>Date: Mon, 16 Nov 2009 19:28:20 +0200
>Subject: [PATCH] qemu: qxl: fix unsafe rom data handling bug #537888

>The guest driver could have change the data inside the QXLRom
>and therefore when the QXLRom was looking on this data it could have
>seen a wrong data (that was changed by the guest).
>Fixing it by adding a shadow_rom and when the qxl device read data
>it look on the shadow_rom that is not accessable by the guest driver.
>Signed-off-by: Izik Eidus <ieidus redhat com>
---
 qemu/hw/qxl.c |   54 +++++++++++++++++++++++++++++++++++-------------------
 1 files changed, 35 insertions(+), 19 deletions(-)
(adapted from the patch post)

Currently qxl device drivers have been provided for both RHEL (xorg-x11-drv-qxl) and Windows (qxldd.dll  qxl.inf  qxl.sys, ...).
However, how can we do to let the guest driver change the data inside the QXLRom so as to test this fix?
Is it possible that we have a test case from the userspace without modifing the guest driver code? I'd appreciate it if you could provide us with one, or tell us how to deal with it.
Thanks in advance.

Comment 8 Izik Eidus 2010-03-02 04:14:14 UTC
if you look on the windows driver, or the X driver
you will see QXLRom structure there,

Just try to set value of the field inside this strcture. and you will see it changed at the host

Thanks.

Comment 9 Keqin Hong 2010-03-02 09:53:03 UTC
(In reply to comment #8)
> if you look on the windows driver, or the X driver
> you will see QXLRom structure there,
> 
> Just try to set value of the field inside this strcture. and you will see it
> changed at the host
> 
> Thanks.    

I have made the guest qxl driver change QXLRom by modifying the xf86-video-qxl-0.0.12/src/qxl_driver.c, recompiling then replacing the qxl_drv.so.

1. Following info from "/var/log/Xorg.0.log" confirmed my pollution to the qxl driver:
"
(II) qxl(0): ram at 0x2b12186e7000; vram at 0x2b121c6e7000; rom at 0x2b121c6e8000
(II) qxl(0): khong test begin:
(II) qxl(0): rom->magic 4f525851
(II) qxl(0): Device version 0.0
(II) qxl(0): Compression level 1, log level 1
(II) qxl(0): Currently using mode #-1, list at 0x34
(II) qxl(0): 14357 io pages at 0x0
(II) qxl(0): 4147200 byte draw area at 0x3815b68
(II) qxl(0): RAM header offset: 0x3ffeb68
(II) qxl(0): --- polluting rom info ---
(II) qxl(0): Compression level 2, log level 2
(II) qxl(0): khong test end:
(II) qxl(0): Setting mode 25 (1024 x 768) (1024 x 768) 0x13023ea0
" 

2. I tried to modify QXLRom by adding "rom->draw_area_size /=2;", but I am not sure it is the right way to do it.
The result was if I opened a text file (e.g. /var/log/Xorg.0.log) using vi, then scrolled the window up and down by typing 'j' and 'k',
spicec got core dump. (If you want, I can provide the backtrace) 

3. So, Izik, can you give me some suggestions on how to pollute the QXLRom the right way? I would appreciate it, if you could, so as to prove this patch fix by adding shadow_rom.

Thank you!

# diff -u xf86-video-qxl-0.0.12_untouched/src/qxl_driver.c xf86-video-qxl-0.0.12/src/qxl_driver.c
--- xf86-video-qxl-0.0.12_untouched/src/qxl_driver.c	2010-02-06 18:10:43.000000000 -0500
+++ xf86-video-qxl-0.0.12/src/qxl_driver.c	2010-03-02 04:47:58.000000000 -0500
@@ -266,7 +266,7 @@
 
     return pScreen->CloseScreen(scrnIndex, pScreen);
 }
-
+/* test here */
 static Bool
 qxl_switch_mode(int scrnIndex, DisplayModePtr p, int flags)
 {
@@ -277,6 +277,25 @@
 
     if (!m)
 	return FALSE;
+/* khong test*/
+struct qxl_rom* rom = qxl->rom;
+xf86DrvMsg(scrnIndex, X_INFO, "khong test begin: \n");
+xf86DrvMsg(scrnIndex, X_INFO, "rom->magic %x\n", rom->magic);
+xf86DrvMsg(scrnIndex, X_INFO, "Device version %d.%d\n",
+                rom->id, rom->update_id);
+xf86DrvMsg(scrnIndex, X_INFO, "Compression level %d, log level %d\n", rom->compression_level, rom->log_level);
+xf86DrvMsg(scrnIndex, X_INFO, "Currently using mode #%d, list at 0x%x\n", rom->mode, rom->modes_offset);
+xf86DrvMsg(scrnIndex, X_INFO, "%d io pages at 0x%x\n", rom->num_io_pages, rom->pages_offset);
+xf86DrvMsg(scrnIndex, X_INFO, "%d byte draw area at 0x%x\n", rom->draw_area_size, rom->draw_area_offset);
+xf86DrvMsg(scrnIndex, X_INFO, "RAM header offset: 0x%x\n", rom->ram_header_offset);
+
+xf86DrvMsg(scrnIndex, X_INFO, "--- polluting rom info --- \n");
+rom->compression_level = rom->compression_level + 1;
+rom->log_level = rom->log_level + 1;
+xf86DrvMsg(scrnIndex, X_INFO, "Compression level %d, log level %d\n", rom->compression_level, rom->log_level);
+rom->draw_area_size /=2;
+xf86DrvMsg(scrnIndex, X_INFO, "khong test end: \n");
+/*end test*/
 
     /* if (debug) */
     xf86DrvMsg (scrnIndex, X_INFO, "Setting mode %d (%d x %d) (%d x %d) %p\n",

Comment 13 errata-xmlrpc 2010-03-30 07:55:03 UTC
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2010-0271.html

Comment 14 Petr Matousek 2010-04-19 08:36:21 UTC
*** Bug 568726 has been marked as a duplicate of this bug. ***

Comment 15 Petr Matousek 2010-04-29 20:01:32 UTC
*** Bug 568727 has been marked as a duplicate of this bug. ***


Note You need to log in before you can comment on or make changes to this bug.