forked from AuroraMiddleware/gtk
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
This commit is contained in:
Colin Walters
parent
faf35d708b
commit
806c04411d
@ -280,6 +280,9 @@ extern gboolean _gdk_disable_multidevice;
|
||||
void _gdk_events_queue (GdkDisplay *display);
|
||||
GdkEvent* _gdk_event_unqueue (GdkDisplay *display);
|
||||
|
||||
void _gdk_event_filter_unref (GdkWindow *window,
|
||||
GdkEventFilter *filter);
|
||||
|
||||
void _gdk_event_emit (GdkEvent *event);
|
||||
GList* _gdk_event_queue_find_first (GdkDisplay *display);
|
||||
void _gdk_event_queue_remove_link (GdkDisplay *display,
|
||||
|
||||
@ -1794,19 +1794,55 @@ gdk_window_ensure_native (GdkWindow *window)
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
/**
|
||||
* _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)
|
||||
{
|
||||
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)
|
||||
{
|
||||
GdkEventFilter *iter_filter = tmp_list->data;
|
||||
GList *node;
|
||||
|
||||
if (iter_filter != filter)
|
||||
continue;
|
||||
|
||||
g_assert (iter_filter->ref_count > 0);
|
||||
|
||||
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)
|
||||
{
|
||||
if (window->filters)
|
||||
{
|
||||
GList *tmp_list;
|
||||
|
||||
for (tmp_list = window->filters; tmp_list; tmp_list = tmp_list->next)
|
||||
g_free (tmp_list->data);
|
||||
|
||||
g_list_free (window->filters);
|
||||
window->filters = NULL;
|
||||
}
|
||||
while (window->filters)
|
||||
_gdk_event_filter_unref (window, window->filters->data);
|
||||
}
|
||||
|
||||
static void
|
||||
@ -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;
|
||||
}
|
||||
|
||||
@ -55,14 +55,17 @@ static GSourceFuncs event_funcs = {
|
||||
static GList *event_sources = NULL;
|
||||
|
||||
static gint
|
||||
gdk_event_apply_filters (XEvent *xevent,
|
||||
GdkEvent *event,
|
||||
GList **filters)
|
||||
gdk_event_apply_filters (XEvent *xevent,
|
||||
GdkEvent *event,
|
||||
GdkWindow *window)
|
||||
{
|
||||
GList *tmp_list;
|
||||
GdkFilterReturn result;
|
||||
|
||||
tmp_list = *filters;
|
||||
if (window == NULL)
|
||||
tmp_list = _gdk_default_filters;
|
||||
else
|
||||
tmp_list = window->filters;
|
||||
|
||||
while (tmp_list)
|
||||
{
|
||||
@ -78,18 +81,12 @@ gdk_event_apply_filters (XEvent *xevent,
|
||||
filter->ref_count++;
|
||||
result = filter->function (xevent, event, filter->data);
|
||||
|
||||
/* get the next node after running the function since the
|
||||
function may add or remove a next node */
|
||||
node = tmp_list;
|
||||
tmp_list = tmp_list->next;
|
||||
/* Protect against unreffing the filter mutating the list */
|
||||
node = tmp_list->next;
|
||||
|
||||
filter->ref_count--;
|
||||
if (filter->ref_count == 0)
|
||||
{
|
||||
*filters = g_list_remove_link (*filters, node);
|
||||
g_list_free_1 (node);
|
||||
g_free (filter);
|
||||
}
|
||||
_gdk_event_filter_unref (window, filter);
|
||||
|
||||
tmp_list = node;
|
||||
|
||||
if (result != GDK_FILTER_CONTINUE)
|
||||
return result;
|
||||
@ -162,8 +159,7 @@ gdk_event_source_translate_event (GdkEventSource *event_source,
|
||||
{
|
||||
/* Apply global filters */
|
||||
|
||||
result = gdk_event_apply_filters (xevent, event,
|
||||
&_gdk_default_filters);
|
||||
result = gdk_event_apply_filters (xevent, event, NULL);
|
||||
|
||||
if (result == GDK_FILTER_REMOVE)
|
||||
{
|
||||
@ -186,7 +182,7 @@ gdk_event_source_translate_event (GdkEventSource *event_source,
|
||||
if (filter_window->filters)
|
||||
{
|
||||
result = gdk_event_apply_filters (xevent, event,
|
||||
&filter_window->filters);
|
||||
filter_window);
|
||||
|
||||
if (result == GDK_FILTER_REMOVE)
|
||||
{
|
||||
|
||||
Loading…
Reference in New Issue
Block a user