gdk: Fix GdkWindowFilter internal refcounting

Running gnome-shell under valgrind, I saw the attached invalid write.
Basically we can destroy a window during event processing, and the old
window_remove_filters simply called g_free() on the filter, ignoring
the refcount.  Then later in event processing we call filter->refcount--,
which is writing to free()d memory.

Fix this by centralizing list mutation and refcount handling inside
a new shared _gdk_window_filter_unref() function, and using that
everywhere.

==13876== Invalid write of size 4
==13876==    at 0x446B181: gdk_event_apply_filters (gdkeventsource.c:86)
==13876==    by 0x446B411: _gdk_events_queue (gdkeventsource.c:188)
==13876==    by 0x44437EF: gdk_display_get_event (gdkdisplay.c:410)
==13876==    by 0x446B009: gdk_event_source_dispatch (gdkeventsource.c:317)
==13876==    by 0x4AB7159: g_main_context_dispatch (gmain.c:2436)
==13876==    by 0x4AB7957: g_main_context_iterate.clone.5 (gmain.c:3087)
==13876==    by 0x4AB806A: g_main_loop_run (gmain.c:3295)
==13876==    by 0x8084D6B: main (main.c:722)
==13876==  Address 0x1658bcac is 12 bytes inside a block of size 16 free'd
==13876==    at 0x4005EAD: free (vg_replace_malloc.c:366)
==13876==    by 0x4ABE515: g_free (gmem.c:263)
==13876==    by 0x444BCC9: window_remove_filters (gdkwindow.c:1873)
==13876==    by 0x4454BA3: _gdk_window_destroy_hierarchy (gdkwindow.c:2043)
==13876==    by 0x447BF6E: gdk_window_destroy_notify (gdkwindow-x11.c:1115)
==13876==    by 0x43588E2: _gtk_socket_windowing_filter_func (gtksocket-x11.c:518)
==13876==    by 0x446B170: gdk_event_apply_filters (gdkeventsource.c:79)
==13876==    by 0x446B411: _gdk_events_queue (gdkeventsource.c:188)
==13876==    by 0x44437EF: gdk_display_get_event (gdkdisplay.c:410)
==13876==    by 0x446B009: gdk_event_source_dispatch (gdkeventsource.c:317)
==13876==    by 0x4AB7159: g_main_context_dispatch (gmain.c:2436)
==13876==    by 0x4AB7957: g_main_context_iterate.clone.5 (gmain.c:3087)

https://bugzilla.gnome.org/show_bug.cgi?id=637464

1 files changed, 45 insertions, 17 deletions

diff --git a/gdk/gdkwindow.c b/gdk/gdkwindow.c
index d5e5e1488d..da50f994d4 100644
--- a/gdk/gdkwindow.c
+++ b/gdk/gdkwindow.c
@@ -1794,22 +1794,58 @@ gdk_window_ensure_native (GdkWindow *window)
   return TRUE;
 }
 
-static void
-window_remove_filters (GdkWindow *window)
+/**
+ * _gdk_event_filter_unref:
+ * @window: A #GdkWindow, or %NULL to be the global window
+ * @filter: A window filter
+ *
+ * Release a reference to @filter.  Note this function may
+ * mutate the list storage, so you need to handle this
+ * if iterating over a list of filters.
+ */
+void
+_gdk_event_filter_unref (GdkWindow       *window,
+			 GdkEventFilter  *filter)
 {
-  if (window->filters)
+  GList **filters;
+  GList *tmp_list;
+
+  if (window == NULL)
+    filters = &_gdk_default_filters;
+  else
+    filters = &window->filters;
+
+  for (tmp_list = *filters; tmp_list; tmp_list = tmp_list->next)
     {
-      GList *tmp_list;
+      GdkEventFilter *iter_filter = tmp_list->data;
+      GList *node;
+
+      if (iter_filter != filter)
+	continue;
 
-      for (tmp_list = window->filters; tmp_list; tmp_list = tmp_list->next)
-	g_free (tmp_list->data);
+      g_assert (iter_filter->ref_count > 0);
 
-      g_list_free (window->filters);
-      window->filters = NULL;
+      filter->ref_count--;
+      if (filter->ref_count != 0)
+	continue;
+
+      node = tmp_list;
+      tmp_list = tmp_list->next;
+
+      *filters = g_list_remove_link (*filters, node);
+      g_free (filter);
+      g_list_free_1 (node);
     }
 }
 
 static void
+window_remove_filters (GdkWindow *window)
+{
+  while (window->filters)
+    _gdk_event_filter_unref (window, window->filters->data);
+}
+
+static void
 update_pointer_info_foreach (GdkDisplay           *display,
                              GdkDevice            *device,
                              GdkPointerWindowInfo *pointer_info,
@@ -2486,16 +2522,8 @@ gdk_window_remove_filter (GdkWindow     *window,
       if ((filter->function == function) && (filter->data == data))
 	{
           filter->flags |= GDK_EVENT_FILTER_REMOVED;
-          filter->ref_count--;
-          if (filter->ref_count != 0)
-            return;
 
-	  if (window)
-	    window->filters = g_list_remove_link (window->filters, node);
-	  else
-	    _gdk_default_filters = g_list_remove_link (_gdk_default_filters, node);
-	  g_list_free_1 (node);
-	  g_free (filter);
+	  _gdk_event_filter_unref (window, filter);
 
 	  return;
 	}