drivemount: fix use-after-free crash on icon theme change#697
Conversation
DriveButton connects drive_button_theme_change to the process-global
default GtkIconTheme ("changed" signal) in drive_button_new() and
drive_button_new_from_mount(), but drive_button_dispose() never
disconnected it. The default icon theme outlives the button, so after a
button is destroyed (e.g. a volume or mount is removed) a later
"changed" emission invokes the callback on freed memory, which then
schedules drive_button_update() via g_idle_add() and crashes in the GTK
main loop with a SIGSEGV inside the DRIVE_IS_BUTTON() type check.
Disconnect the handler in drive_button_dispose(). Confirmed under
AddressSanitizer: the dangling-handler path reports heap-use-after-free
without this change and runs cleanly with it.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
@Cigydd this change looks fine to me, but I would like your commit message attribution to follow MATE's contribution guidelines: https://github.com/mate-desktop/mate-applets?tab=contributing-ov-file#attribution tl;dr: Clean up the commit message to be more readable, and change the co-authorship like this (or something reasonably similar to it): - Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
+ Assisted-By: Claude:claude-opus-4.8 |
|
I looked over this thankfully very simple code change, basically the same as what a human author would have
written. I guess the real question is why wasn't this included when the applet was first written? Or maybe it got
lost somewhere?
|
lukefromdc
left a comment
There was a problem hiding this comment.
This builds fine and stops the crash, which I was able to verify by first ejecting a drive with this applet in the panel, then changing icon themes. As expected, that took down the whole panel with all applets in-process.
Applying this change to disconnect the signal handler stops the crash.
Holding off on approval until the attribution is fixed, but if I wrote this code myself the only likely change would be the wording of the comment. I checked the docs for this over at
https://docs.gtk.org/gobject/func.signal_handlers_disconnect_by_func.html
DriveButtonconnectsdrive_button_theme_changeto the process-global defaultGtkIconTheme"changed"signal indrive_button_new()anddrive_button_new_from_mount(), butdrive_button_dispose()never disconnectedit. Because the default icon theme outlives the button, after a button is
destroyed (e.g. a volume or mount is removed) a later
"changed"emissioninvokes the callback on freed memory and schedules
drive_button_update()viag_idle_add(), crashing in the GTK main loop (SIGSEGV inside theDRIVE_IS_BUTTON()type check).On affected systems this applet has been crashing repeatedly for months, around
USB device plug/unplug.
Disconnect the handler in
drive_button_dispose():Verified with AddressSanitizer: a harness that destroys a
DriveButtonand thenemits
"changed"on the default icon theme reportsheap-use-after-freeindrive_button_queue_update(viadrive_button_theme_change) without thischange, and runs cleanly with it.
Fixes #696.
🤖 Generated with Claude Code