Re: qemu-kvm 0.12.2 VNC segfault

Re: qemu-kvm 0.12.2 VNC segfault

[Avi Kivity]

On 02/21/2010 07:23 PM, Chris Webb wrote:
> Some sort of race where a client disconnects and is removed from the client
> list while the vnc_refresh() loop is iterating over it, maybe?
>    

Looks like c727a05459, and high time for 0.12.3.  Anthony?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: qemu-kvm 0.12.2 VNC segfault

[Chris Webb]

Avi Kivity <avi@redhat.com> writes:

> On 02/21/2010 07:23 PM, Chris Webb wrote:
> >Some sort of race where a client disconnects and is removed from the client
> >list while the vnc_refresh() loop is iterating over it, maybe?
> 
> Looks like c727a05459, and high time for 0.12.3.  Anthony?

Ah yes, looks like this was exactly the case that commit was trying to
prevent. Thanks!

Best wishes,

Chris.

Re: qemu-kvm 0.12.2 VNC segfault

[Anthony Liguori]

On 02/22/2010 02:54 AM, Avi Kivity wrote:
> On 02/21/2010 07:23 PM, Chris Webb wrote:
>> Some sort of race where a client disconnects and is removed from the 
>> client
>> list while the vnc_refresh() loop is iterating over it, maybe?
>
> Looks like c727a05459, and high time for 0.12.3.  Anthony?

Indeed.

Regards,

Anthony Liguori

Another VNC crash, qemu-kvm-0.12.3

[Chris Webb]

We've just seen another VNC related qemu-kvm crash, this time an arithmetic
exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3.

  [...]
  1423     if (vs->absolute) { 
  1424         kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
  1425                         y * 0x7FFF / (ds_get_height(vs->ds) - 1),
  1426                         dz, buttons);
  1427     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) { 
  1428         x -= 0x7FFF;
  [...]

and sure enough:

  (gdb) p vs->ds->surface->width
  $1 = 9
  (gdb) p vs->ds->surface->height
  $2 = 1

What a 9x1 display surface is doing on this guest is a mystery to me, but you
definitely can't divide by one less than its height!

  (gdb) p *vs
  $3 = {csock = 19, ds = 0x1c60fa0, dirty = {{4294967295, 4294967295, 4294967295, 4294967295, 
        4294967295} <repeats 2048 times>}, vd = 0x26a0110, need_update = 1, force_update = 0, features = 67, 
    absolute = 1, last_x = -1, last_y = -1, vnc_encoding = 5, tight_quality = 9 '\t', tight_compression = 9 '\t', 
    major = 3, minor = 8, challenge = "¹{\177\226\200kÕjéPñÄA¤o)", output = {capacity = 925115, offset = 0, 
      buffer = 0x28ba4b0 ""}, input = {capacity = 5120, offset = 6, buffer = 0x28b90a0 "\005"}, 
    write_pixels = 0x4bb9e0 <vnc_write_pixels_generic>, send_hextile_tile = 0x4bcdf0 <send_hextile_tile_generic_32>, 
    clientds = {flags = 0 '\0', width = 800, height = 600, linesize = 3200, data = 0x7fcd00ab6010 "", pf = {
        bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 0, gmask = 0, bmask = 0, 
        amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'ÿ', 
        gmax = 255 'ÿ', bmax = 255 'ÿ', amax = 255 'ÿ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b', 
        abits = 8 '\b'}}, audio_cap = 0x0, as = {freq = 44100, nchannels = 2, fmt = AUD_FMT_S16, endianness = 0}, 
    read_handler = 0x4beac0 <protocol_client_msg>, read_handler_expect = 6, modifiers_state = '\0' <repeats 255 times>, 
    zlib = {capacity = 0, offset = 0, buffer = 0x0}, zlib_tmp = {capacity = 0, offset = 0, buffer = 0x0}, 
    zlib_stream = {{next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, 
        state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, 
        avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, 
        zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, 
        next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, 
        data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, 
        avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, 
        adler = 0, reserved = 0}}, next = 0x0}

  (gdb) p *vs->ds
  $4 = {surface = 0x1c81f40, opaque = 0x26a0110, gui_timer = 0x0, allocator = 0x8199d0, listeners = 0x1c95fa0, 
    mouse_set = 0, cursor_define = 0, next = 0x0}

  (gdb) p *vs->ds->surface
  $5 = {flags = 2 '\002', width = 9, height = 1, linesize = 36, data = 0x7fcd00ab6010 "", pf = {
      bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 16711680, gmask = 65280, 
      bmask = 255, amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'ÿ', 
      gmax = 255 'ÿ', bmax = 255 'ÿ', amax = 255 'ÿ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b', abits = 8 '\b'}}

Cheers,

Chris.

Re: Another VNC crash, qemu-kvm-0.12.3

[Anthony Liguori]

On 03/01/2010 12:14 PM, Chris Webb wrote:
> We've just seen another VNC related qemu-kvm crash, this time an arithmetic
> exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3.
>
>    [...]
>    1423     if (vs->absolute) {
>    1424         kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
>    1425                         y * 0x7FFF / (ds_get_height(vs->ds) - 1),
>    1426                         dz, buttons);
>    1427     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
>    1428         x -= 0x7FFF;
>    [...]
>
> and sure enough:
>
>    (gdb) p vs->ds->surface->width
>    $1 = 9
>    (gdb) p vs->ds->surface->height
>    $2 = 1
>
> What a 9x1 display surface is doing on this guest is a mystery to me, but you
> definitely can't divide by one less than its height!
>    

Can you reproduce this reliably?  If so, what's the procedure?

BTW, I'd suggest filing this at http://bugs.launchpad.net/qemu

Regards,

Anthony Liguori

>    (gdb) p *vs
>    $3 = {csock = 19, ds = 0x1c60fa0, dirty = {{4294967295, 4294967295, 4294967295, 4294967295,
>          4294967295}<repeats 2048 times>}, vd = 0x26a0110, need_update = 1, force_update = 0, features = 67,
>      absolute = 1, last_x = -1, last_y = -1, vnc_encoding = 5, tight_quality = 9 '\t', tight_compression = 9 '\t',
>      major = 3, minor = 8, challenge = "¹{\177\226\200kÕjéPñÄA¤o)", output = {capacity = 925115, offset = 0,
>        buffer = 0x28ba4b0 ""}, input = {capacity = 5120, offset = 6, buffer = 0x28b90a0 "\005"},
>      write_pixels = 0x4bb9e0<vnc_write_pixels_generic>, send_hextile_tile = 0x4bcdf0<send_hextile_tile_generic_32>,
>      clientds = {flags = 0 '\0', width = 800, height = 600, linesize = 3200, data = 0x7fcd00ab6010 "", pf = {
>          bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 0, gmask = 0, bmask = 0,
>          amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'ÿ',
>          gmax = 255 'ÿ', bmax = 255 'ÿ', amax = 255 'ÿ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b',
>          abits = 8 '\b'}}, audio_cap = 0x0, as = {freq = 44100, nchannels = 2, fmt = AUD_FMT_S16, endianness = 0},
>      read_handler = 0x4beac0<protocol_client_msg>, read_handler_expect = 6, modifiers_state = '\0'<repeats 255 times>,
>      zlib = {capacity = 0, offset = 0, buffer = 0x0}, zlib_tmp = {capacity = 0, offset = 0, buffer = 0x0},
>      zlib_stream = {{next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0,
>          state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0,
>          avail_in = 0, total_in = 0, next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0,
>          zfree = 0, opaque = 0x0, data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0,
>          next_out = 0x0, avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0,
>          data_type = 0, adler = 0, reserved = 0}, {next_in = 0x0, avail_in = 0, total_in = 0, next_out = 0x0,
>          avail_out = 0, total_out = 0, msg = 0x0, state = 0x0, zalloc = 0, zfree = 0, opaque = 0x0, data_type = 0,
>          adler = 0, reserved = 0}}, next = 0x0}
>
>    (gdb) p *vs->ds
>    $4 = {surface = 0x1c81f40, opaque = 0x26a0110, gui_timer = 0x0, allocator = 0x8199d0, listeners = 0x1c95fa0,
>      mouse_set = 0, cursor_define = 0, next = 0x0}
>
>    (gdb) p *vs->ds->surface
>    $5 = {flags = 2 '\002', width = 9, height = 1, linesize = 36, data = 0x7fcd00ab6010 "", pf = {
>        bits_per_pixel = 32 ' ', bytes_per_pixel = 4 '\004', depth = 24 '\030', rmask = 16711680, gmask = 65280,
>        bmask = 255, amask = 0, rshift = 16 '\020', gshift = 8 '\b', bshift = 0 '\0', ashift = 24 '\030', rmax = 255 'ÿ',
>        gmax = 255 'ÿ', bmax = 255 'ÿ', amax = 255 'ÿ', rbits = 8 '\b', gbits = 8 '\b', bbits = 8 '\b', abits = 8 '\b'}}
>
> Cheers,
>
> Chris.
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>    

Re: Another VNC crash, qemu-kvm-0.12.3

[Chris Webb]

Anthony Liguori <anthony@codemonkey.ws> writes:

> On 03/01/2010 12:14 PM, Chris Webb wrote:
> >We've just seen another VNC related qemu-kvm crash, this time an arithmetic
> >exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3.
> >
> >   [...]
> >   1423     if (vs->absolute) {
> >   1424         kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
> >   1425                         y * 0x7FFF / (ds_get_height(vs->ds) - 1),
> >   1426                         dz, buttons);
> >   1427     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
> >   1428         x -= 0x7FFF;
> >   [...]
> >
> >and sure enough:
> >
> >   (gdb) p vs->ds->surface->width
> >   $1 = 9
> >   (gdb) p vs->ds->surface->height
> >   $2 = 1
> >
> >What a 9x1 display surface is doing on this guest is a mystery to me, but you
> >definitely can't divide by one less than its height!
> 
> Can you reproduce this reliably?  If so, what's the procedure?

No, I'm afraid not, although I have had a thorough play myself with a variety
of VNC clients in an attempt to reproduce.

The background here is that we're running a public hosting service where
customers can install and run their own OSes on their own qemu-kvm virtual
machines. I don't even know what VNC client (if any) was connected at the
time. I only see the core dump if the qemu-kvm crashes.

Of course, if the screen width or height is 1, it doesn't really matter what
the value of the mouse position for the click is, so something as simple as

diff --git a/vnc.c b/vnc.c
--- a/vnc.c
+++ b/vnc.c
@@ -1421,8 +1421,10 @@
         dz = 1;
 
     if (vs->absolute) {
-        kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
-                        y * 0x7FFF / (ds_get_height(vs->ds) - 1),
+        kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
+                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
+                        ds_get_height(vs->ds) > 1 ?
+                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
                         dz, buttons);
     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
         x -= 0x7FFF;

will fix the symptom: the division by zero. The underlying cause of a 9x1
display surface is a bit mysterious though.

Cheers,

Chris.

Re: [Qemu-devel] Re: Another VNC crash, qemu-kvm-0.12.3

[Alexander Graf]

On 05.03.2010, at 17:52, Chris Webb wrote:

> Anthony Liguori <anthony@codemonkey.ws> writes:
> 
>> On 03/01/2010 12:14 PM, Chris Webb wrote:
>>> We've just seen another VNC related qemu-kvm crash, this time an arithmetic
>>> exception at vnc.c:1424 in the newly release qemu-kvm 0.12.3.
>>> 
>>>  [...]
>>>  1423     if (vs->absolute) {
>>>  1424         kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
>>>  1425                         y * 0x7FFF / (ds_get_height(vs->ds) - 1),
>>>  1426                         dz, buttons);
>>>  1427     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
>>>  1428         x -= 0x7FFF;
>>>  [...]
>>> 
>>> and sure enough:
>>> 
>>>  (gdb) p vs->ds->surface->width
>>>  $1 = 9
>>>  (gdb) p vs->ds->surface->height
>>>  $2 = 1
>>> 
>>> What a 9x1 display surface is doing on this guest is a mystery to me, but you
>>> definitely can't divide by one less than its height!
>> 
>> Can you reproduce this reliably?  If so, what's the procedure?
> 
> No, I'm afraid not, although I have had a thorough play myself with a variety
> of VNC clients in an attempt to reproduce.
> 
> The background here is that we're running a public hosting service where
> customers can install and run their own OSes on their own qemu-kvm virtual
> machines. I don't even know what VNC client (if any) was connected at the
> time. I only see the core dump if the qemu-kvm crashes.
> 
> Of course, if the screen width or height is 1, it doesn't really matter what
> the value of the mouse position for the click is, so something as simple as
> 
> diff --git a/vnc.c b/vnc.c
> --- a/vnc.c
> +++ b/vnc.c
> @@ -1421,8 +1421,10 @@
>         dz = 1;
> 
>     if (vs->absolute) {
> -        kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
> -                        y * 0x7FFF / (ds_get_height(vs->ds) - 1),
> +        kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
> +                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
> +                        ds_get_height(vs->ds) > 1 ?
> +                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
>                         dz, buttons);
>     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
>         x -= 0x7FFF;
> 
> will fix the symptom: the division by zero. The underlying cause of a 9x1
> display surface is a bit mysterious though.

Is it? When booting the screen gets resized to something like 9x1 for a few ms. Try putting debug code in the resize callback - you'll see it.

Alex

Re: [Qemu-devel] Re: Another VNC crash, qemu-kvm-0.12.3

[Chris Webb]

Alexander Graf <agraf@suse.de> writes:

> On 05.03.2010, at 17:52, Chris Webb wrote:
>
> > Of course, if the screen width or height is 1, it doesn't really matter what
> > the value of the mouse position for the click is, so something as simple as
> > 
> > diff --git a/vnc.c b/vnc.c
> > --- a/vnc.c
> > +++ b/vnc.c
> > @@ -1421,8 +1421,10 @@
> >         dz = 1;
> > 
> >     if (vs->absolute) {
> > -        kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
> > -                        y * 0x7FFF / (ds_get_height(vs->ds) - 1),
> > +        kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
> > +                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
> > +                        ds_get_height(vs->ds) > 1 ?
> > +                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
> >                         dz, buttons);
> >     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
> >         x -= 0x7FFF;
> > 
> > will fix the symptom: the division by zero. The underlying cause of a 9x1
> > display surface is a bit mysterious though.
> 
> Is it? When booting the screen gets resized to something like 9x1 for a
> few ms. Try putting debug code in the resize callback - you'll see it.

Ah, okay. In that case, this patch could well be the correct fix rather than
just a work-around. I'll have a look for any other places in vnc.c that
might do a similar division-by-zero for small screen sizes at the same
point.

Best wishes,

Chris.

[PATCH] Fix SIGFPE for vnc display of width/height = 1

[Chris Webb]

During boot, the screen gets resized to height 1 and a mouse click at this
point will cause a division by zero when calculating the absolute pointer
position from the pixel (x, y). Return a click in the middle of the screen
instead in this case.

Signed-off-by: Chris Webb <chris@arachsys.com>
---
 vnc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/vnc.c b/vnc.c
index 01353a9..676a707 100644
--- a/vnc.c
+++ b/vnc.c
@@ -1457,8 +1457,10 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
         dz = 1;
 
     if (vs->absolute) {
-        kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
-                        y * 0x7FFF / (ds_get_height(vs->ds) - 1),
+        kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
+                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
+                        ds_get_height(vs->ds) > 1 ?
+                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
                         dz, buttons);
     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
         x -= 0x7FFF;
-- 
1.7.0.1

Re: [Qemu-devel] [PATCH] Fix SIGFPE for vnc display of width/height = 1

[Chris Webb]

Chris Webb <chris@arachsys.com> writes:

> During boot, the screen gets resized to height 1 and a mouse click at this
> point will cause a division by zero when calculating the absolute pointer
> position from the pixel (x, y). Return a click in the middle of the screen
> instead in this case.

I think this probably ought to be a candidate for 0.12-stable too. We're
seeing these crashes for real from time-to-time so it's not just a
theoretical problem.

Cheers,

Chris.

Re: [Qemu-devel] [PATCH] Fix SIGFPE for vnc display of width/height = 1

[Anthony Liguori]

On 03/08/2010 08:34 AM, Chris Webb wrote:
> During boot, the screen gets resized to height 1 and a mouse click at this
> point will cause a division by zero when calculating the absolute pointer
> position from the pixel (x, y). Return a click in the middle of the screen
> instead in this case.
>
> Signed-off-by: Chris Webb<chris@arachsys.com>
>    
Applied.  Thanks.

Regards,

Anthony Liguori
> ---
>   vnc.c |    6 ++++--
>   1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/vnc.c b/vnc.c
> index 01353a9..676a707 100644
> --- a/vnc.c
> +++ b/vnc.c
> @@ -1457,8 +1457,10 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
>           dz = 1;
>
>       if (vs->absolute) {
> -        kbd_mouse_event(x * 0x7FFF / (ds_get_width(vs->ds) - 1),
> -                        y * 0x7FFF / (ds_get_height(vs->ds) - 1),
> +        kbd_mouse_event(ds_get_width(vs->ds)>  1 ?
> +                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
> +                        ds_get_height(vs->ds)>  1 ?
> +                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
>                           dz, buttons);
>       } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
>           x -= 0x7FFF;
>    

Re: [Qemu-devel] [PATCH] Fix SIGFPE for vnc display of width/height = 1

[Alexander Graf]

Anthony Liguori wrote:
> On 03/08/2010 08:34 AM, Chris Webb wrote:
>> During boot, the screen gets resized to height 1 and a mouse click at
>> this
>> point will cause a division by zero when calculating the absolute
>> pointer
>> position from the pixel (x, y). Return a click in the middle of the
>> screen
>> instead in this case.
>>
>> Signed-off-by: Chris Webb<chris@arachsys.com>
>>    
> Applied.  Thanks.

Also queued it to stable?


Alex