Patchwork Re: Another VNC crash, qemu-kvm-0.12.3

login
register
mail settings
Submitter Chris Webb
Date March 5, 2010, 4:52 p.m.
Message ID <20100305165209.GL29711@arachsys.com>
Download mbox | patch
Permalink /patch/47013/
State New
Headers show

Comments

Chris Webb - March 5, 2010, 4:52 p.m.
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


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

Cheers,

Chris.
Alexander Graf - March 5, 2010, 7:57 p.m.
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
Chris Webb - March 6, 2010, 8:53 a.m.
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

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;