Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Gerd Hoffmann]

On Fr, 2016-09-09 at 07:13 +, Gonglei (Arei) wrote:
> Hi Gerd,
> 
> Can you pls pick up this patch? thanks

Added to UI queue, thanks.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Gonglei (Arei)]

Hi Gerd,

Can you pls pick up this patch? thanks

Regards,
-Gonglei


> -Original Message-
> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Sent: Friday, September 02, 2016 8:39 PM
> To: Marc-André Lureau
> Cc: Gonglei (Arei); QEMU Developers; Huangweidong (C); Gerd Hoffmann
> Subject: Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of
> SIGSEGV
> 
> On 2 September 2016 at 13:34, Marc-André Lureau
> <marcandre.lur...@gmail.com> wrote:
> > Hi
> >
> > On Fri, Sep 2, 2016 at 3:04 PM Gonglei <arei.gong...@huawei.com> wrote:
> >
> >>
> >> > It looks like this is not a regression from 2.7, perhaps it should be
> >> post-poned?
> >> >
> >> Yes, it's not a regression from 2.7, but it indeed is a serious bug and
> >> the fix is harmless. :)
> >>
> >>
> > The timing is bad. Unless Gerd or a maintainer sends a pull request today
> > with it, it's probably not going to make it in 2.7 (due today according to
> > planning).
> 
> For a non-regression this would have had to be sent at least a
> week ago to have had a chance of getting into 2.7. I would only accep
> anything into 2.7 now if it was an absolute release-breaker
> (eg "crashes on startup for 50% of users"); this is a long way from
> that. 2.8 and cc qemu-stable, please.
> 
> thanks
> -- PMM

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Gonglei (Arei)]

okay, thank you, guys.
发件人：Peter Maydell
收件人：Marc-André Lureau,
抄送：龚磊,qemu-devel,黄伟栋,Gerd Hoffmann,
时间：2016-09-02 20:39:52
主题:Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

On 2 September 2016 at 13:34, Marc-André Lureau
<marcandre.lur...@gmail.com> wrote:
> Hi
>
> On Fri, Sep 2, 2016 at 3:04 PM Gonglei <arei.gong...@huawei.com> wrote:
>
>>
>> > It looks like this is not a regression from 2.7, perhaps it should be
>> post-poned?
>> >
>> Yes, it's not a regression from 2.7, but it indeed is a serious bug and
>> the fix is harmless. :)
>>
>>
> The timing is bad. Unless Gerd or a maintainer sends a pull request today
> with it, it's probably not going to make it in 2.7 (due today according to
> planning).

For a non-regression this would have had to be sent at least a
week ago to have had a chance of getting into 2.7. I would only accep
anything into 2.7 now if it was an absolute release-breaker
(eg "crashes on startup for 50% of users"); this is a long way from
that. 2.8 and cc qemu-stable, please.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Peter Maydell]

On 2 September 2016 at 13:34, Marc-André Lureau
 wrote:
> Hi
>
> On Fri, Sep 2, 2016 at 3:04 PM Gonglei  wrote:
>
>>
>> > It looks like this is not a regression from 2.7, perhaps it should be
>> post-poned?
>> >
>> Yes, it's not a regression from 2.7, but it indeed is a serious bug and
>> the fix is harmless. :)
>>
>>
> The timing is bad. Unless Gerd or a maintainer sends a pull request today
> with it, it's probably not going to make it in 2.7 (due today according to
> planning).

For a non-regression this would have had to be sent at least a
week ago to have had a chance of getting into 2.7. I would only accep
anything into 2.7 now if it was an absolute release-breaker
(eg "crashes on startup for 50% of users"); this is a long way from
that. 2.8 and cc qemu-stable, please.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Marc-André Lureau]

Hi

On Fri, Sep 2, 2016 at 3:04 PM Gonglei  wrote:

>
> > It looks like this is not a regression from 2.7, perhaps it should be
> post-poned?
> >
> Yes, it's not a regression from 2.7, but it indeed is a serious bug and
> the fix is harmless. :)
>
>
The timing is bad. Unless Gerd or a maintainer sends a pull request today
with it, it's probably not going to make it in 2.7 (due today according to
planning). Furthermore, since it's not a regression, I think it's okay to
post-pone. It will be backported and part of future -stable releases.
-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Gonglei]

On 2016/9/2 16:38, Marc-André Lureau wrote:
> Hi
> 
> On Fri, Sep 2, 2016 at 8:00 AM Gonglei  > wrote:
> 
> The backtrace is:
> 
> 0x7f0b75cdf880 in pixman_image_get_stride () from 
> /lib64/libpixman-1.so.0
> 0x7f0b77bcb3cf 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
> 0x7f0b77bbcc35 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
> 0x7f0b77bbda4e 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
> 0x7f0b77ac0980 in cirrus_do_copy (h=, w= out>, 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
> 0x7f0b77965cfb in memory_region_write_accessor (mr=0x7f0b7b096e40, 
> addr=320, value=, size=1, shift= out>,mask=, attrs=...) at 
> /root/rpmbuild/BUILD/master/qemu/memory.c:525
> 0x7f0b77963f59 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
> 0x7f0b77968315 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
> 0x7f0b779256a9 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
> 0x7f0b77925c1d 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
> 0x7f0b77962f53 in kvm_cpu_exec (cpu=cpu@entry=0x7f0b79fcc2d0) at 
> /root/rpmbuild/BUILD/master/qemu/kvm-all.c:1965
> 0x7f0b77950cc6 in qemu_kvm_cpu_thread_fn (arg=0x7f0b79fcc2d0) at 
> /root/rpmbuild/BUILD/master/qemu/cpus.c:1078
> 0x7f0b744b3dc5 in start_thread (arg=0x7f0b69a27700) at 
> pthread_create.c:308
> 0x7f0b70d3d66d 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.
> 
> 
> Reviewed-by: Marc-André Lureau 
> 
Thanks.

> (It would be great if you had a reproducer)
> 

1.using VNC Viewer client tool.
2.using SUSE 11.3 as guest VM with graphic console.
3.connecting vnc as soon as possible after starting the VM.

I get the below information before qemu crash.

[New Thread 0x7ffee93ff700 (LWP 18570)]
[Switching to Thread 0x7fffea305700 (LWP 17105)]

Breakpoint 1, vnc_client_io_error (vs=0x581025a0, ret=-2, 
errp=0x7fffea3045b0) at ui/vnc.c:1262
1262vnc_disconnect_start(vs);
(gdb) bt
#0  vnc_client_io_error (vs=0x581025a0, ret=-2, errp=0x7fffea3045b0) at 
ui/vnc.c:1262
#1  0x559fce2b in vnc_client_write_buf (vs=0x581025a0, 
data=, datalen=) at ui/vnc.c:1302
#2  0x559fcee6 in vnc_client_write_plain (vs=) at 
ui/vnc.c:1333
#3  vnc_client_write_locked (vs=0x581025a0) at ui/vnc.c:1366
#4  0x559fd901 in vnc_flush (vs=0x581025a0) at ui/vnc.c:1557
#5  0x559fe6ea in vnc_copy (h=210, w=472, dst_y=261, dst_x=222, 
src_y=279, src_x=276, vs=0x581025a0) at ui/vnc.c:886
#6  vnc_dpy_copy (dcl=0x570b0c50, src_x=276, src_y=279, dst_x=222, 
dst_y=261, w=472, h=210) at ui/vnc.c:965
#7  0x559efc35 in dpy_gfx_copy (con=0x570a6030, 
src_x=src_x@entry=276, src_y=src_y@entry=279, dst_x=dst_x@entry=222,
dst_y=dst_y@entry=261, w=472, h=210) at ui/console.c:1575
#8  0x559f0a4e in qemu_console_copy (con=, 
src_x=src_x@entry=276, src_y=src_y@entry=279, dst_x=dst_x@entry=222,
dst_y=dst_y@entry=261, w=, h=) at 
ui/console.c:2111
#9  

Re: [Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Marc-André Lureau]

Hi

On Fri, Sep 2, 2016 at 8:00 AM Gonglei  wrote:

> The backtrace is:
>
> 0x7f0b75cdf880 in pixman_image_get_stride () from
> /lib64/libpixman-1.so.0
> 0x7f0b77bcb3cf 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
> 0x7f0b77bbcc35 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
> 0x7f0b77bbda4e 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
> 0x7f0b77ac0980 in cirrus_do_copy (h=, w= out>, 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
> 0x7f0b77965cfb in memory_region_write_accessor (mr=0x7f0b7b096e40,
> addr=320, value=, size=1, shift= out>,mask=, attrs=...) at
> /root/rpmbuild/BUILD/master/qemu/memory.c:525
> 0x7f0b77963f59 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
> 0x7f0b77968315 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
> 0x7f0b779256a9 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
> 0x7f0b77925c1d 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
> 0x7f0b77962f53 in kvm_cpu_exec (cpu=cpu@entry=0x7f0b79fcc2d0) at
> /root/rpmbuild/BUILD/master/qemu/kvm-all.c:1965
> 0x7f0b77950cc6 in qemu_kvm_cpu_thread_fn (arg=0x7f0b79fcc2d0) at
> /root/rpmbuild/BUILD/master/qemu/cpus.c:1078
> 0x7f0b744b3dc5 in start_thread (arg=0x7f0b69a27700) at
> pthread_create.c:308
> 0x7f0b70d3d66d 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.
>

Reviewed-by: Marc-André Lureau 

(It would be great if you had a reproducer)

It looks like this is not a regression from 2.7, perhaps it should be
post-poned?

Cc: Gerd Hoffmann 
> Cc: Daniel P. Berrange 
> Reported-by: Yanying Zhuang 
> Signed-off-by: Gonglei 
> ---
>  ui/vnc.c | 4 
>  1 file changed, 4 insertions(+)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index d1087c9..76a3273 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);
> --
> 1.7.12.4
>
>
>
> --
Marc-André Lureau

[Qemu-devel] [PATCH for-2.7] vnc: fix qemu crash because of SIGSEGV

[Gonglei]

The backtrace is:

0x7f0b75cdf880 in pixman_image_get_stride () from /lib64/libpixman-1.so.0
0x7f0b77bcb3cf 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
0x7f0b77bbcc35 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
0x7f0b77bbda4e 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
0x7f0b77ac0980 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
0x7f0b77965cfb in memory_region_write_accessor (mr=0x7f0b7b096e40, 
addr=320, value=, size=1, shift=,mask=, attrs=...) at /root/rpmbuild/BUILD/master/qemu/memory.c:525
0x7f0b77963f59 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
0x7f0b77968315 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
0x7f0b779256a9 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
0x7f0b77925c1d 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
0x7f0b77962f53 in kvm_cpu_exec (cpu=cpu@entry=0x7f0b79fcc2d0) at 
/root/rpmbuild/BUILD/master/qemu/kvm-all.c:1965
0x7f0b77950cc6 in qemu_kvm_cpu_thread_fn (arg=0x7f0b79fcc2d0) at 
/root/rpmbuild/BUILD/master/qemu/cpus.c:1078
0x7f0b744b3dc5 in start_thread (arg=0x7f0b69a27700) at pthread_create.c:308
0x7f0b70d3d66d 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 
---
 ui/vnc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ui/vnc.c b/ui/vnc.c
index d1087c9..76a3273 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);
-- 
1.7.12.4