From 3665bd5da733f56fb4cca659beee0bfc62867ec6 Mon Sep 17 00:00:00 2001 From: Mike Fleetwood Date: Mon, 20 Sep 2021 18:35:01 +0100 Subject: [PATCH] Fix crash scrolling quickly in the drive selection combobox (#165) A user reported that scrolling the mouse wheel quickly crashes GParted [1]. The minimum setup required to reproduce this crash is 3 drives partitioned like this: sda - any sdb - some unallocated space sdc - no unallocated space Then move the pointer over the drive selection combobox and scroll the mouse wheel quickly downwards. The sequence of events goes like this: 1. The first scroll wheel down event causes the device combobox selection to change to the second entry (sdb), which calls combo_devices_changed() -> Refresh_Visual(). 2. Refresh_Visual() sets selected_partition_ptr to point to the largest unallocated space partition object in sdb. 3. The first Gtk event processing loop in Refresh_Visual() comes across the next scroll wheel down event. This changes the selection to the third entry (sdc), which makes a recursive call to combo_devices_changed() -> Refresh_Visual(). 4. Refresh_Visual() sets selected_partition_ptr to NULL as sdc has no unallocated space and returns. 5. The first call to Refresh_Visual() resumes after the first Gtk event loop, continuing the processing for drive sdb. As sdb has a largest unallocated space partition (largest_unalloc_size >= 0), DrawingAreaVisualDisk::set_selected() is called with the now NULL selected_partition_ptr. 6. One of the DrawingAreaVisualDisk::set_selected() methods dereferences the NULL pointer, crashing GParted. As a comment says of selected_partition_ptr "Lifetime: Valid until the next call to Refresh_Visual()." It just wasn't expected that the next call to Refresh_Visual() would be half way through Refresh_Visual() itself. Considered but rejected fixes: 1. Remove automatic selection of the largest unallocated space. Removes functionality. 2. Return at the top of Refresh_Visual() when called recursively. This causes GParted to not update the main window with the latest selected drive. In the above example the combobox would show sdc, but the main window graphic and partition list would have only been updated once showing sdb, the first scrolled selection. Fix by only having a single Gtk event processing loop at the end of Refresh_Visual() with the optional calls to select the largest unallocated partition before that loop. This makes it impossible to call the ::set_selected() methods with selected_partition_ptr modified by a recursive call. This fix reverts this earlier commit: 0fb8cce6992f238b78272b603b69e070cfd46a00 Reduce flashing redraw from automatic partition selection (#696149) That commit was in GParted 0.20.0 when Gtk 2 was still used. Re-testing now doesn't see any flashing redrawing from the automatic partition selection, with GParted currently using Gtk 3. [1] Debian bug #991998 - gparted segfaults if scrolling quickly the device dropdown list https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=991998 Closes #165 - GParted segfaults if scrolling quickly in the device dropdown --- src/Win_GParted.cc | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/Win_GParted.cc b/src/Win_GParted.cc index b4cf5051..24e364dc 100644 --- a/src/Win_GParted.cc +++ b/src/Win_GParted.cc @@ -1102,23 +1102,20 @@ void Win_GParted::Refresh_Visual() set_valid_operations() ; - // Process Gtk events to redraw visuals with reloaded partition details - while ( Gtk::Main::events_pending() ) - Gtk::Main::iteration(); - if ( largest_unalloc_size >= 0 ) { - // Flashing redraw work around. Inform visuals of selection of the - // largest unallocated partition after drawing those visuals above. + // Inform visuals of selection of the largest unallocated partition. drawingarea_visualdisk.set_selected( selected_partition_ptr ); treeview_detail.set_selected( selected_partition_ptr ); - - // Process Gtk events to draw selection - while ( Gtk::Main::events_pending() ) - Gtk::Main::iteration(); } + + // Process Gtk events to redraw visuals with reloaded partition details. + while (Gtk::Main::events_pending()) + Gtk::Main::iteration(); + } + // Confirms that the pointer points to one of the partition objects in the vector of // displayed partitions, Win_GParted::display_partitions[]. // Usage: g_assert( valid_display_partition_ptr( my_partition_ptr ) );