Merge lp:~bregma/geis/lp-1077376 into lp:geis

[Stephen M. Webb]

Avoid dereferencing a freed pointer when more than one V1 gesture class is subscribed (lp: #1077376).

Avoids dereferencing a freed pointer when more than one V1 gesture class is subscribed (lp: #1077376).

No new tests are required: current existing tests were failing and now pass with this fix.

Evince no longer crashes with this fix applied.

[Daniel d'Andrada (dandrader)]

Looks ok, although I would be happier to have a regular test to cover this instead of a xorg-gtest one (which is slow and unreliable). I have to confess xorg-gtest-based tests stopped working for me long ago so I didn't bother running them anymore. I just write and check the regular ones.

review: Approve

=== modified file 'libgeis/geis_subscription.c'
--- libgeis/geis_subscription.c	2012-09-05 14:59:39 +0000
+++ libgeis/geis_subscription.c	2012-11-13 02:57:18 +0000
@@ -337,6 +337,7 @@
   if (sub->sub_backend_token)
   {
     status = geis_backend_token_deactivate(sub->sub_backend_token, sub);
+    sub->sub_backend_token = NULL;
   }
   return status;
 }

=== modified file 'libgeis/geis_v1.c'
--- libgeis/geis_v1.c	2012-07-24 16:04:12 +0000
+++ libgeis/geis_v1.c	2012-11-13 02:57:18 +0000
@@ -619,19 +619,7 @@
   {
     const char **g;
 
-    if (device_id != 0)
-    {
-      geis_debug("subscribing device %d for the following gestures:", device_id);
-      result = geis_filter_add_term(instance->window_filter,
-                          GEIS_FILTER_DEVICE,
-                          GEIS_DEVICE_ATTRIBUTE_ID, GEIS_FILTER_OP_EQ,
-                          device_id, NULL);
-      if (result != GEIS_STATUS_SUCCESS)
-      {
-        geis_error("error adding device filter term");
-      }
-    }
-
+    geis_debug("subscribing device %d for the following gestures:", device_id);
     for (g = gesture_list; *g; ++g)
     {
       GeisV1AttrMap v1attr;
@@ -664,6 +652,17 @@
 	      geis_error("error adding gesture class filter term");
 	    }
 
+            if (device_id != 0)
+            {
+              result = geis_filter_add_term(filter,
+                                  GEIS_FILTER_DEVICE, GEIS_DEVICE_ATTRIBUTE_ID, GEIS_FILTER_OP_EQ,
+                                  device_id, NULL);
+              if (result != GEIS_STATUS_SUCCESS)
+              {
+                geis_error("error adding device filter term");
+              }
+            }
+
 	    result = geis_subscription_add_filter(instance->subscription, filter); 
 	    if (result != GEIS_STATUS_SUCCESS)
 	    {
@@ -674,14 +673,6 @@
 	}
       }
     }
-
-    /* remove the window-id-only filter since it allows ALL gesture classes */
-    result = geis_subscription_remove_filter(instance->subscription, 
-                                             instance->window_filter);
-    if (result != GEIS_STATUS_SUCCESS)
-    {
-      geis_warning("error removing V1 window filter");
-    }
   }
   return result;
 }
@@ -696,6 +687,16 @@
 {
   GeisStatus result = GEIS_UNKNOWN_ERROR;
 
+  /**
+   * If there is no window filter, the instance has already been subscribed to
+   * devices and gestures so skip.
+   */
+  if (NULL == instance->window_filter)
+  {
+    geis_warning("instance has been subscribed twice");
+    return GEIS_STATUS_SUCCESS;
+  }
+
   memcpy(&instance->gesture_funcs, funcs, sizeof(GeisGestureFuncs));
   instance->gesture_cookie = cookie;
 
@@ -719,6 +720,23 @@
       }
     }
   }
+
+  /*
+   * If there are specific gesture classes, remove the window-id-only filter since it allows
+   * ALL gesture classes.  It will have been replaced by device-specific or
+   * class-specific filters on the same window.
+   */
+  if (gesture_list && *gesture_list)
+  {
+    result = geis_subscription_remove_filter(instance->subscription, 
+                                             instance->window_filter);
+    if (result != GEIS_STATUS_SUCCESS)
+    {
+      geis_warning("error removing V1 window filter");
+    }
+    instance->window_filter = NULL;
+  }
+
   result = geis_subscription_activate(instance->subscription);
   return result;
 }