diff mbox series

vga: check the validation of memory addr when draw text

Message ID 20171225022835.23236-1-linzhecheng@huawei.com
State New
Headers show
Series vga: check the validation of memory addr when draw text | expand

Commit Message

linzhecheng Dec. 25, 2017, 2:28 a.m. UTC
Start a vm with qemu-kvm -enable-kvm -vnc :66 -smp 1 -m 1024 -hda redhat_5.11.qcow2  -device pcnet -vga cirrus,
then use VNC client to connect to VM, and excute the code below in guest OS will lead to qemu crash:

int main()
 {
    iopl(3);
    srand(time(NULL));
    int a,b;
    while(1){
	a = rand()%0x100;
	b = 0x3c0 + (rand()%0x20);
        outb(a,b);
    }
    return 0;
}

The backtrace is:
#0  0x000055defdf28dd1 in vga_draw_text (s=0x55deffe19a80, full_update=1)
    at /mnt/sdb/lzc/code/open/qemu/hw/display/vga.c:1283
#1  0x000055defdf2a371 in vga_update_display (opaque=0x55deffe19a80)
    at /mnt/sdb/lzc/code/open/qemu/hw/display/vga.c:1766
#2  0x000055defe28098e in graphic_hw_update (con=0x55deffeeb770) at ui/console.c:263
#3  0x000055defe29360f in vnc_refresh (dcl=0x55deffc54860) at ui/vnc.c:2855
#4  0x000055defe2842fe in dpy_refresh (s=0x55deffeeb700) at ui/console.c:1595
#5  0x000055defe2806ca in gui_update (opaque=0x55deffeeb700) at ui/console.c:201
#6  0x000055defe3ca875 in timerlist_run_timers (timer_list=0x55deff420100) at util/qemu-timer.c:536
#7  0x000055defe3ca8bd in qemu_clock_run_timers (type=QEMU_CLOCK_REALTIME) at util/qemu-timer.c:547
#8  0x000055defe3cac83 in qemu_clock_run_all_timers () at util/qemu-timer.c:662
#9  0x000055defe3cb430 in main_loop_wait (nonblocking=0) at util/main-loop.c:521
#10 0x000055defe029838 in main_loop () at vl.c:1951
#11 0x000055defe031720 in main (argc=16, argv=0x7ffe5fb600e8, envp=0x7ffe5fb60170) at vl.c:4867

The above code is writing the registers of VGA randomly.
We can write VGA CRT controller registers index 0x0C or 0x0D 
(which is the start address register) to modify the 
the display memory address of the upper left pixel 
or character of the screen. The address may be out of the 
range of vga ram. So we should check the validation of memory address 
when reading or writing it to avoid segfault.

Signed-off-by: linzhecheng <linzhecheng@huawei.com>

Change-Id: Ib7466361b18e0a232fc068aad50d2113701786ab

Comments

Prasad Pandit Jan. 11, 2018, 10:41 a.m. UTC | #1
+-- On Mon, 25 Dec 2017, linzhecheng wrote --+
| --- a/hw/display/vga.c
| +++ b/hw/display/vga.c
| @@ -1279,6 +1279,10 @@ static void vga_draw_text(VGACommonState *s, int full_update)
|          cx_min = width;
|          cx_max = -1;
|          for(cx = 0; cx < width; cx++) {
| +            if (src + sizeof(uint16_t) > s->vram_ptr + s->vram_size) {
| +                printf("src is out of the range of vga ram.\n");
| +                return;
| +             }
|              ch_attr = *(uint16_t *)src;

This does fix the OOB access and segfault issue. Maybe it could 'break;' 
instead of 'return;' with no printf(...)?

Thank you.
--
Prasad J Pandit / Red Hat Product Security Team
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
diff mbox series

Patch

diff --git a/hw/display/vga.c b/hw/display/vga.c
index a0412000a5..c265572bf3 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1279,6 +1279,10 @@  static void vga_draw_text(VGACommonState *s, int full_update)
         cx_min = width;
         cx_max = -1;
         for(cx = 0; cx < width; cx++) {
+            if (src + sizeof(uint16_t) > s->vram_ptr + s->vram_size) {
+                printf("src is out of the range of vga ram.\n");
+                return;
+             }
             ch_attr = *(uint16_t *)src;
             if (full_update || ch_attr != *ch_attr_ptr || src == cursor_ptr) {
                 if (cx < cx_min)