Skip to content
GitLab
  • Menu
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
  • Sign in / Register
  • phosh phosh
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 124
    • Issues 124
    • List
    • Boards
    • Service Desk
    • Milestones
  • Merge requests 0
    • Merge requests 0
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • Librem5
  • phoshphosh
  • Merge requests
  • !792

home: Fix use after free

  • Review changes

  • Download
  • Email patches
  • Plain diff
Merged Guido Gunther requested to merge guido.gunther/phosh:use-after-free into master Apr 20, 2021
  • Overview 6
  • Commits 2
  • Pipelines 4
  • Changes 4

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 Apr 20, 2021 by Guido Gunther
Assignee
Assign to
Reviewer
Request review from
Time tracking
Source branch: use-after-free