From bba4e1b591531c087fce4ae501dc1ca299d8fb42 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 11 Aug 2016 15:23:26 +0100 Subject: [PATCH 1/4] ui/curses.c: Ensure we don't read off the end of curses2qemu array Coverity spots that there is no bounds check before we access the curses2qemu[] array. Add one, bringing this code path into line with the one that looks up entries in curses2keysym[]. In theory getch() shouldn't return out of range keycodes, but it's better not to assume this. Signed-off-by: Peter Maydell Message-id: 1470925407-23850-2-git-send-email-peter.maydell@linaro.org Signed-off-by: Gerd Hoffmann --- ui/curses.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ui/curses.c b/ui/curses.c index b47558956c..f1f886c968 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -317,7 +317,10 @@ static void curses_refresh(DisplayChangeListener *dcl) qemu_input_event_send_key_delay(0); } } else { - keysym = curses2qemu[chr]; + keysym = -1; + if (chr < CURSES_KEYS) { + keysym = curses2qemu[chr]; + } if (keysym == -1) keysym = chr; From 99a9ef44dca4be93f60c38d83a79eaaf8c56548a Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Thu, 11 Aug 2016 15:23:27 +0100 Subject: [PATCH 2/4] ui/curses.c: Clean up nextchr logic Coverity identifies that at the top of the while(1) loop in curses_refresh() the variable nextchr is always ERR, and so the else case of the first if() is dead code. Remove this dead code, and narrow the scope of the nextchr variable to the place where it's used. (This confused logic has been present since the curses code was added to QEMU in 2008.) Signed-off-by: Peter Maydell Message-id: 1470925407-23850-3-git-send-email-peter.maydell@linaro.org Signed-off-by: Gerd Hoffmann --- ui/curses.c | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/ui/curses.c b/ui/curses.c index f1f886c968..d06f724879 100644 --- a/ui/curses.c +++ b/ui/curses.c @@ -181,7 +181,7 @@ static kbd_layout_t *kbd_layout = NULL; static void curses_refresh(DisplayChangeListener *dcl) { - int chr, nextchr, keysym, keycode, keycode_alt; + int chr, keysym, keycode, keycode_alt; curses_winch_check(); @@ -195,15 +195,9 @@ static void curses_refresh(DisplayChangeListener *dcl) graphic_hw_text_update(NULL, screen); - nextchr = ERR; while (1) { /* while there are any pending key strokes to process */ - if (nextchr == ERR) - chr = getch(); - else { - chr = nextchr; - nextchr = ERR; - } + chr = getch(); if (chr == ERR) break; @@ -224,13 +218,12 @@ static void curses_refresh(DisplayChangeListener *dcl) /* alt key */ if (keycode == 1) { - nextchr = getch(); + int nextchr = getch(); if (nextchr != ERR) { chr = nextchr; keycode_alt = ALT; - keycode = curses2keycode[nextchr]; - nextchr = ERR; + keycode = curses2keycode[chr]; if (keycode != -1) { keycode |= ALT; From 93ca519ec45cb26133da59d65575e839c62e9144 Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Mon, 1 Aug 2016 12:06:59 +0800 Subject: [PATCH 3/4] qemu-options.hx: correct spice options streaming-video default document value to 'off' since f1d3e58, the code had changed the default value to 'off', so this patch make document and code are consistent. Signed-off-by: Li Zhijian Message-id: 1470024419-10886-1-git-send-email-lizhijian@cn.fujitsu.com Signed-off-by: Gerd Hoffmann --- qemu-options.hx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qemu-options.hx b/qemu-options.hx index a71aaf8ea8..70dfe986a2 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1145,7 +1145,7 @@ Configure wan image compression (lossy for slow links). Default is auto. @item streaming-video=[off|all|filter] -Configure video stream detection. Default is filter. +Configure video stream detection. Default is off. @item agent-mouse=[on|off] Enable/disable passing mouse events via vdagent. Default is on. From 3e10c3ecfcaf604d8b400d6e463e1a186ce97d9b Mon Sep 17 00:00:00 2001 From: Gonglei Date: Fri, 2 Sep 2016 11:58:18 +0800 Subject: [PATCH 4/4] vnc: fix qemu crash because of SIGSEGV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The backtrace is: 0x00007f0b75cdf880 in pixman_image_get_stride () from /lib64/libpixman-1.so.0 0x00007f0b77bcb3cf in vnc_server_fb_stride (vd=0x7f0b7a1a2bb0) at ui/vnc.c:680 vnc_dpy_copy (dcl=0x7f0b7a1a2c00, src_x=224, src_y=263, dst_x=319, dst_y=363, w=1, h=1) at ui/vnc.c:915 0x00007f0b77bbcc35 in dpy_gfx_copy (con=0x7f0b7a146210, src_x=src_x@entry=224, src_y=src_y@entry=263, dst_x=dst_x@entry=319, dst_y=dst_y@entry=363, w=1, h=1) at ui/console.c:1575 0x00007f0b77bbda4e in qemu_console_copy (con=, src_x=src_x@entry=224, src_y=src_y@entry=263, dst_x=dst_x@entry=319, dst_y=dst_y@entry=363, w=, h=) at ui/console.c:2111 0x00007f0b77ac0980 in cirrus_do_copy (h=, w=, src=, dst=, s=0x7f0b7b086090) at hw/display/cirrus_vga.c:774 cirrus_bitblt_videotovideo_copy (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:793 cirrus_bitblt_videotovideo (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:915 cirrus_bitblt_start (s=0x7f0b7b086090) at hw/display/cirrus_vga.c:1056 0x00007f0b77965cfb in memory_region_write_accessor (mr=0x7f0b7b096e40, addr=320, value=, size=1, shift=,mask=, attrs=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:525 0x00007f0b77963f59 in access_with_adjusted_size (addr=addr@entry=320, value=value@entry=0x7f0b69a268d8, size=size@entry=4, access_size_min=, access_size_max=, access=access@entry=0x7f0b77965c80 , mr=mr@entry=0x7f0b7b096e40, attrs=attrs@entry=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:591 0x00007f0b77968315 in memory_region_dispatch_write (mr=mr@entry=0x7f0b7b096e40, addr=addr@entry=320, data=18446744073709551362, size=size@entry=4, attrs=attrs@entry=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:1262 0x00007f0b779256a9 in address_space_write_continue (mr=0x7f0b7b096e40, l=4, addr1=320, len=4, buf=0x7f0b77713028 "\002\377\377\377", attrs=..., addr=4273930560, as=0x7f0b7827d280 ) at /root/rpmbuild/BUILD/master/qemu/exec.c:2544 address_space_write (as=, addr=, attrs=..., buf=, len=) at /root/rpmbuild/BUILD/master/qemu/exec.c:2601 0x00007f0b77925c1d in address_space_rw (as=, addr=, attrs=..., attrs@entry=..., buf=buf@entry=0x7f0b77713028 "\002\377\377\377", len=, is_write=) at /root/rpmbuild/BUILD/master/qemu/exec.c:2703 0x00007f0b77962f53 in kvm_cpu_exec (cpu=cpu@entry=0x7f0b79fcc2d0) at /root/rpmbuild/BUILD/master/qemu/kvm-all.c:1965 0x00007f0b77950cc6 in qemu_kvm_cpu_thread_fn (arg=0x7f0b79fcc2d0) at /root/rpmbuild/BUILD/master/qemu/cpus.c:1078 0x00007f0b744b3dc5 in start_thread (arg=0x7f0b69a27700) at pthread_create.c:308 0x00007f0b70d3d66d in clone () from /lib64/libc.so.6 The code path while meeting segfault: vnc_dpy_copy vnc_update_client vnc_disconnect_finish [while vnc_disconnect_start() is invoked because somethins wrong] vnc_update_server_surface vd->server = NULL; vnc_server_fb_stride pixman_image_get_stride(vd->server) Let's add a non-NULL check before calling vnc_server_fb_stride() to avoid segmentation fault. Cc: Gerd Hoffmann Cc: Daniel P. Berrange Reported-by: Yanying Zhuang Signed-off-by: Gonglei Reviewed-by: Marc-André Lureau Message-id: 1472788698-120964-1-git-send-email-arei.gonglei@huawei.com Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ui/vnc.c b/ui/vnc.c index d1087c93a5..76a3273e0b 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -911,6 +911,10 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl, } } + if (!vd->server) { + /* no client connected */ + return; + } /* do bitblit op on the local surface too */ pitch = vnc_server_fb_stride(vd); src_row = vnc_server_fb_ptr(vd, src_x, src_y);