[PATCH 19/24] dix: always copy grabs, don't reference them

Peter Hutterer peter.hutterer at who-t.net
Thu May 9 22:30:54 PDT 2013


Introduced in xorg-server-1.13.99.901-2-g9ad0fdb. Storing the grab pointer
in the listener turns out to be a bad idea. If the grab is not an active
grab or an implicit grab, the pointer stored is the one to the grab attached
on the window. This grab may be removed if the client calls UngrabButton or
similar while the touch is still active, leaving a dangling pointer.

To avoid this, copy the grab wherever we need to reference it later. This
is also what we do for pointer/keyboard grabs, where we copy the grab as
soon as it becomes active.

Reported-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
---
 Xi/exevents.c |  7 +++++--
 dix/events.c  |  3 ++-
 dix/touch.c   | 19 +++++++++++++++++--
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Xi/exevents.c b/Xi/exevents.c
index 2feddd3..ae2e5ae 100644
--- a/Xi/exevents.c
+++ b/Xi/exevents.c
@@ -1429,8 +1429,11 @@ DeliverTouchEmulatedEvent(DeviceIntPtr dev, TouchPointInfoPtr ti,
          */
         if (!devgrab && dev->deviceGrab.grab && dev->deviceGrab.implicitGrab) {
             TouchListener *l;
+            GrabPtr g;
 
             devgrab = dev->deviceGrab.grab;
+            g = AllocGrab(devgrab);
+            BUG_WARN(!g);
 
             *dev->deviceGrab.sync.event = ev->device_event;
 
@@ -1439,8 +1442,8 @@ DeliverTouchEmulatedEvent(DeviceIntPtr dev, TouchPointInfoPtr ti,
              * event selection. Thus, we update the last listener in the array.
              */
             l = &ti->listeners[ti->num_listeners - 1];
-            l->listener = devgrab->resource;
-            l->grab = devgrab;
+            l->listener = g->resource;
+            l->grab = g;
             //l->resource_type = RT_NONE;
 
             if (devgrab->grabtype != XI2 || devgrab->type != XI_TouchBegin)
diff --git a/dix/events.c b/dix/events.c
index cd2d939..76bc555 100644
--- a/dix/events.c
+++ b/dix/events.c
@@ -1442,7 +1442,8 @@ UpdateTouchesForGrab(DeviceIntPtr mouse)
                 listener->type = LISTENER_POINTER_GRAB;
             else
                 listener->type = LISTENER_GRAB;
-            listener->grab = grab;
+            FreeGrab(listener->grab);
+            listener->grab = AllocGrab(grab);
         }
     }
 }
diff --git a/dix/touch.c b/dix/touch.c
index 01a554e..fb218d4 100644
--- a/dix/touch.c
+++ b/dix/touch.c
@@ -365,6 +365,8 @@ TouchBeginTouch(DeviceIntPtr dev, int sourceid, uint32_t touchid,
 void
 TouchEndTouch(DeviceIntPtr dev, TouchPointInfoPtr ti)
 {
+    int i;
+
     if (ti->emulate_pointer) {
         GrabPtr grab;
 
@@ -376,6 +378,9 @@ TouchEndTouch(DeviceIntPtr dev, TouchPointInfoPtr ti)
         }
     }
 
+    for (i = 0; i < ti->num_listeners; i++)
+        TouchRemoveListener(ti, ti->listeners[0].listener);
+
     ti->active = FALSE;
     ti->pending_finish = FALSE;
     ti->sprite.spriteTraceGood = 0;
@@ -692,15 +697,23 @@ void
 TouchAddListener(TouchPointInfoPtr ti, XID resource, int resource_type,
                  enum InputLevel level, enum TouchListenerType type,
                  enum TouchListenerState state, WindowPtr window,
-                 GrabPtr grab)
+                 const GrabPtr grab)
 {
+    GrabPtr g = NULL;
+
+    /* We need a copy of the grab, not the grab itself since that may be
+     * deleted by a UngrabButton request and leaves us with a dangling
+     * pointer */
+    if (grab)
+        g = AllocGrab(grab);
+
     ti->listeners[ti->num_listeners].listener = resource;
     ti->listeners[ti->num_listeners].resource_type = resource_type;
     ti->listeners[ti->num_listeners].level = level;
     ti->listeners[ti->num_listeners].state = state;
     ti->listeners[ti->num_listeners].type = type;
     ti->listeners[ti->num_listeners].window = window;
-    ti->listeners[ti->num_listeners].grab = grab;
+    ti->listeners[ti->num_listeners].grab = g;
     if (grab)
         ti->num_grabs++;
     ti->num_listeners++;
@@ -725,6 +738,7 @@ TouchRemoveListener(TouchPointInfoPtr ti, XID resource)
             continue;
 
         if (listener->grab) {
+            FreeGrab(listener->grab);
             listener->grab = NULL;
             ti->num_grabs--;
         }
@@ -734,6 +748,7 @@ TouchRemoveListener(TouchPointInfoPtr ti, XID resource)
         ti->num_listeners--;
         ti->listeners[ti->num_listeners].listener = 0;
         ti->listeners[ti->num_listeners].state = LISTENER_AWAITING_BEGIN;
+
         return TRUE;
     }
     return FALSE;
-- 
1.8.1.4



More information about the xorg-devel mailing list