Commit df144043 authored by Guido Gunther's avatar Guido Gunther Committed by Sebastian Krzyszkowiak
Browse files

home: Fix use after free



add_keybindings() kept the GActionEntry around but not the 'name' string
of these. Those were freed by with overview_bindings and
app_view_bindings. When PhoshHome gets destroyed we iterated over
self->actions referencing already freed string names.

To avoid this don't track GActionEntries but rather just the action
names which also avoids reconstructing the GActionEntry in
PhoshGnomeShell:remove_action_entries().

This avoids the following valgrind error:

==14994== Invalid read of size 1
==14994==    at 0x56623D0: g_str_hash (ghash.c:2335)
==14994==    by 0x5660D6A: g_hash_table_lookup_node (ghash.c:473)
==14994==    by 0x5660D6A: g_hash_table_lookup (ghash.c:1513)
==14994==    by 0x54CDCBB: g_simple_action_group_remove_action (gsimpleactiongroup.c:234)
==14994==    by 0x17F480: phosh_shell_remove_global_keyboard_action_entries (shell.c:1287)
==14994==    by 0x168E2E: phosh_home_dispose (home.c:349)
==14994==    by 0x55E2438: g_object_run_dispose (gobject.c:1226)
==14994==    by 0x17CA10: panels_dispose (shell.c:271)
==14994==    by 0x17CA10: phosh_shell_dispose (shell.c:346)
==14994==    by 0x55E10A2: g_object_unref (gobject.c:3465)
==14994==    by 0x55E10A2: g_object_unref (gobject.c:3395)
==14994==    by 0x12B197: glib_autoptr_clear_GObject (gobject-autocleanups.h:27)
==14994==    by 0x12B197: glib_autoptr_clear_PhoshShell (shell.h:57)
==14994==    by 0x12B197: glib_autoptr_cleanup_PhoshShell (shell.h:57)
==14994==    by 0x12B197: main (main.c:80)
==14994==  Address 0xde089f0 is 80 bytes inside a block of size 96 in arena "client"

This was spotted on a regular phosh shutdown but would also trigger when
moving the primary display which recreates PhoshHome.
Signed-off-by: Guido Gunther's avatarGuido Günther <guido.gunther@puri.sm>
parent b932eca4
Pipeline #66896 passed with stages
in 34 minutes and 4 seconds
......@@ -67,15 +67,12 @@ typedef struct _AcceleratorInfo {
} AcceleratorInfo;
static void
remove_action_entries (const gchar *accelerator)
remove_action_entries (gchar *accelerator)
{
const GActionEntry action_entries[] = {
{ accelerator, accelerator_activated_action, },
};
GStrv action_names = (char*[]){ accelerator, NULL };
phosh_shell_remove_global_keyboard_action_entries (phosh_shell_get_default (),
action_entries,
G_N_ELEMENTS (action_entries));
action_names);
}
static void
......
......@@ -67,7 +67,7 @@ struct _PhoshHome
PhoshHomeState state;
/* Keybinding */
GArray *actions;
GStrv action_names;
GSettings *settings;
/* osk button */
......@@ -253,28 +253,38 @@ toggle_application_view_action (GSimpleAction *action, GVariant *param, gpointer
static void
add_keybindings (PhoshHome *self)
{
g_auto (GStrv) overview_bindings = NULL;
g_auto (GStrv) app_view_bindings = NULL;
GStrv overview_bindings;
GStrv app_view_bindings;
GPtrArray *action_names = g_ptr_array_new ();
g_autoptr (GSettings) settings = g_settings_new (KEYBINDINGS_SCHEMA_ID);
g_autoptr (GArray) actions = g_array_new (FALSE, TRUE, sizeof (GActionEntry));
overview_bindings = g_settings_get_strv (settings, KEYBINDING_KEY_TOGGLE_OVERVIEW);
for (int i = 0; i < g_strv_length (overview_bindings); i++) {
GActionEntry entry = { overview_bindings[i],
toggle_overview_action, };
g_array_append_val (self->actions, entry);
g_array_append_val (actions, entry);
g_ptr_array_add (action_names, overview_bindings[i]);
}
/* Free GStrv container but keep individual strings for action_names */
g_free (overview_bindings);
app_view_bindings = g_settings_get_strv (settings, KEYBINDING_KEY_TOGGLE_APPLICATION_VIEW);
for (int i = 0; i < g_strv_length (app_view_bindings); i++) {
GActionEntry entry = { app_view_bindings[i],
toggle_application_view_action, };
g_array_append_val (self->actions, entry);
g_array_append_val (actions, entry);
g_ptr_array_add (action_names, app_view_bindings[i]);
}
/* Free GStrv container but keep individual strings for action_names */
g_free (app_view_bindings);
g_ptr_array_add (action_names, NULL);
phosh_shell_add_global_keyboard_action_entries (phosh_shell_get_default (),
(GActionEntry*)self->actions->data,
self->actions->len,
(GActionEntry*) actions->data,
actions->len,
self);
self->action_names = (GStrv) g_ptr_array_free (action_names, FALSE);
}
......@@ -286,9 +296,8 @@ on_keybindings_changed (PhoshHome *self,
/* For now just redo all keybindings */
g_debug ("Updating keybindings");
phosh_shell_remove_global_keyboard_action_entries (phosh_shell_get_default (),
(GActionEntry*)self->actions->data,
self->actions->len);
g_array_set_size (self->actions, 0);
self->action_names);
g_clear_pointer (&self->action_names, g_strfreev);
add_keybindings (self);
}
......@@ -345,11 +354,10 @@ phosh_home_dispose (GObject *object)
g_clear_object (&self->settings);
if (self->actions) {
if (self->action_names) {
phosh_shell_remove_global_keyboard_action_entries (phosh_shell_get_default (),
(GActionEntry*)self->actions->data,
self->actions->len);
g_clear_pointer (&self->actions, g_array_unref);
self->action_names);
g_clear_pointer (&self->action_names, g_strfreev);
}
G_OBJECT_CLASS (phosh_home_parent_class)->dispose (object);
......@@ -414,7 +422,6 @@ phosh_home_init (PhoshHome *self)
{
self->state = PHOSH_HOME_STATE_FOLDED;
self->animation.progress = 1.0;
self->actions = g_array_new (FALSE, TRUE, sizeof (GActionEntry));
self->settings = g_settings_new (KEYBINDINGS_SCHEMA_ID);
gtk_widget_init_template (GTK_WIDGET (self));
......
......@@ -1273,9 +1273,7 @@ phosh_shell_add_global_keyboard_action_entries (PhoshShell *self,
void
phosh_shell_remove_global_keyboard_action_entries (PhoshShell *self,
const GActionEntry *entries,
gint n_entries)
GStrv action_names)
{
PhoshShellPrivate *priv;
......@@ -1283,9 +1281,9 @@ phosh_shell_remove_global_keyboard_action_entries (PhoshShell *self,
priv = phosh_shell_get_instance_private (self);
g_return_if_fail (priv->keyboard_events);
for (int i = 0; i < n_entries; i++) {
for (int i = 0; i < g_strv_length (action_names); i++) {
g_action_map_remove_action (G_ACTION_MAP (priv->keyboard_events),
entries[i].name);
action_names[i]);
}
}
......
......@@ -93,8 +93,7 @@ void phosh_shell_add_global_keyboard_action_entries (PhoshShell
gint n_entries,
gpointer user_data);
void phosh_shell_remove_global_keyboard_action_entries (PhoshShell *self,
const GActionEntry *actions,
gint n_entries);
GStrv action_names);
gboolean phosh_shell_is_session_active (PhoshShell *self);
GdkAppLaunchContext *phosh_shell_get_app_launch_context (PhoshShell *self);
PhoshShellStateFlags phosh_shell_get_state (PhoshShell *self);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment