Skip to content

home: Fix use after free

Guido Gunther requested to merge guido.gunther/phosh:use-after-free into master

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.

Since i screwed that up befor and there already was a leak in this area (26740806) i'd welcome a review.

Note that although unlikely it might trigger things like #421 when moving the primary display would be involved.

Signed-off-by: Guido Günther guido.gunther@puri.sm

Edited by Guido Gunther

Merge request reports