[PATCH] prevent X crash on pressing multimedia buttons

[Peter Hutterer]

On Mon, Mar 26, 2012 at 04:40:06PM +0200, Michal Suchanek wrote:
> Hello,
> 
> this is basically a backport of
> 2416ee4a015068359807a10f433e8c54192c78a9 to 1.11 branch which fixes X
> server crash on queuing pointer events on devices without axis.
> 
> On 1.11 this is only triggered under special circumstances (eg. when
> Xscreensaver unlock dialog is active).
> 
> It fixes the issue for me.
> 
> I only hit the problem that valuator is NULL.
> 
> Not sure if an event for an axis not present on a device which does
> have some axis can be queued. There are checks for that but not tested
> because such event are not generated in my setup.
> 
> Please review
> 
> Thanks
> 
> Michal

> From 9dd65337fdd705bf74d44073118b48f34793ea71 Mon Sep 17 00:00:00 2001
> From: Michal Suchanek <hramrach at gmail.com>
> Date: Wed, 29 Feb 2012 14:43:16 +0100
> Subject: [PATCH] Avoid crash on button events on device without valuators.
> 
> Signed-off-by: Michal Suchanek <hramrach at gmail.com>
> ---
>  dix/getevents.c |   13 +++++++++----
>  1 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/dix/getevents.c b/dix/getevents.c
> index 058fa8a..958c6d6 100644
> --- a/dix/getevents.c
> +++ b/dix/getevents.c
> @@ -198,6 +198,8 @@ set_valuators(DeviceIntPtr dev, DeviceEvent* event, ValuatorMask *mask)
>  {
>      int i;
>  
> +    if (!dev->valuator)
> +        return;
>      /* Set the data to the previous value for unset absolute axes. The values
>       * may be used when sent as part of an XI 1.x valuator event. */
>      for (i = 0; i < valuator_mask_size(mask); i++)
> @@ -205,13 +207,15 @@ set_valuators(DeviceIntPtr dev, DeviceEvent* event, ValuatorMask *mask)
>          if (valuator_mask_isset(mask, i))
>          {
>              SetBit(event->valuators.mask, i);
> -            if (valuator_get_mode(dev, i) == Absolute)
> +            if ((dev->valuator->numAxes <  i) ||
> +                (valuator_get_mode(dev, i) == Absolute))
>                  SetBit(event->valuators.mode, i);
>              event->valuators.data[i] = valuator_mask_get(mask, i);
>              event->valuators.data_frac[i] =
>                  dev->last.remainder[i] * (1 << 16) * (1 << 16);
>          }
> -        else if (valuator_get_mode(dev, i) == Absolute)
> +        else if ((dev->valuator->numAxes <  i) &&
> +            (valuator_get_mode(dev, i) == Absolute))
>              event->valuators.data[i] = dev->valuator->axisVal[i];
>      }
>  }

wouldn't it be simpler just to add the condition to the for loop?
given that the device wouldn't have any valuators to begin with, we don't
need to set the event mask/data and just continue with the root/event
coordinates.

Cheers,
  Peter


> @@ -640,7 +644,7 @@ clipAxis(DeviceIntPtr pDev, int axisNum, int *val)
>  {
>      AxisInfoPtr axis;
>  
> -    if (axisNum >= pDev->valuator->numAxes)
> +    if (!pDev->valuator || axisNum >= pDev->valuator->numAxes)
>          return;
>  
>      axis = pDev->valuator->axes + axisNum;
> @@ -1185,7 +1189,8 @@ GetPointerEvents(InternalEvent *events, DeviceIntPtr pDev, int type, int buttons
>  
>      if (flags & POINTER_ABSOLUTE)
>      {
> -        if (flags & POINTER_SCREEN) /* valuators are in screen coords */
> +        if ((flags & POINTER_SCREEN) && /* valuators are in screen coords */
> +             pDev->valuator && (pDev->valuator->numAxes >= 2))
>          {
>              int scaled;
>  
> -- 
> 1.7.9.1
> 

> _______________________________________________
> xorg-devel at lists.x.org: X.Org development
> Archives: http://lists.x.org/archives/xorg-devel
> Info: http://lists.x.org/mailman/listinfo/xorg-devel