[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [PATCH] dix: fix crash on XI 1.x grabs on disabled devices. (#54934)
From:       Chase Douglas <chase.douglas () ubuntu ! com>
Date:       2012-09-29 18:22:06
Message-ID: CAKjFQhuinEZcLuaqdsmORML9dEnJ6p_QWh56Qki2J4bzApFphg () mail ! gmail ! com
[Download RAW message or body]

On Thu, Sep 27, 2012 at 6:56 PM, Peter Hutterer
<peter.hutterer@who-t.net> wrote:
> If the device is disabled, the sprite window is NULL and dereferencing
> crashes the server.
>
> This is only triggered for XI 1.x grabs (ProcXGrabDevice) as XI2 grabs would
> trigger another code path, creating a sprite for the disabled device as if
> detaching it (which is wrong and fixed with this patch too).
>
> Grabbing a disabled device doesn't make sense as it won't send events
> anyway. However, the protocol specs do not prohibit it, so we need to keep
> it working.
> Luckily, oldWin is only used for focus out events, which aren't necessary
> given that the device is disabled.
>
> X.Org Bug 54934 <http://bugs.freedesktop.org/show_bug.cgi?id=54934>
>
> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
>  dix/events.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/dix/events.c b/dix/events.c
> index d1931af..96778f7 100644
> --- a/dix/events.c
> +++ b/dix/events.c
> @@ -1555,11 +1555,13 @@ ActivateKeyboardGrab(DeviceIntPtr keybd, GrabPtr grab, TimeStamp time,
>      WindowPtr oldWin;
>
>      /* slave devices need to float for the duration of the grab. */
> -    if (grab->grabtype == XI2 &&
> +    if (grab->grabtype == XI2 && keybd->enabled &&
>          !(passive & ImplicitGrabMask) && !IsMaster(keybd))
>          DetachFromMaster(keybd);
>
> -    if (grabinfo->grab)
> +    if (!keybd->enabled)
> +        oldWin = NULL;
> +    else if (grabinfo->grab)
>          oldWin = grabinfo->grab->window;
>      else if (keybd->focus)
>          oldWin = keybd->focus->win;
> @@ -1571,7 +1573,8 @@ ActivateKeyboardGrab(DeviceIntPtr keybd, GrabPtr grab, TimeStamp time,
>          oldWin = keybd->focus->win;
>      if (keybd->valuator)
>          keybd->valuator->motionHintWindow = NullWindow;
> -    DoFocusEvents(keybd, oldWin, grab->window, NotifyGrab);
> +    if (oldWin)
> +        DoFocusEvents(keybd, oldWin, grab->window, NotifyGrab);
>      if (syncEvents.playingEvents)
>          grabinfo->grabTime = syncEvents.time;
>      else

Looks reasonably correct to me.

Reviewed-by: Chase Douglas <chase.douglas@ubuntu.com>
_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
[prev in list] [next in list] [prev in thread] [next in thread]