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:
    0fb8cce699
    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
This commit is contained in:
Mike Fleetwood 2021-09-20 18:35:01 +01:00 committed by Curtis Gedak
parent 49188a39b8
commit 3665bd5da7
1 changed files with 7 additions and 10 deletions

View File

@ -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 ) );