[PATCH] prevent X crash on pressing multimedia buttons

[Michal Suchanek]

On 16 April 2012 01:40, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> 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.

The event valuators are set in the loop.

I am not sure if those values can be used under some circumstances or not.

However, they are not set for button-only devices that have no
valuator and there is no problem so I guess it's OK to ignore as much
processing as possible.

Attaching an updated (server-1.11 branch) patch.

Thanks

Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Avoid-crash-on-button-events-on-device-without-valua.patch
Type: application/octet-stream
Size: 1674 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20120416/78d3fe84/attachment.obj>