[gparted] Fix crash scrolling quickly in the drive selection combobox (#165)



commit 3665bd5da733f56fb4cca659beee0bfc62867ec6
Author: Mike Fleetwood <mike fleetwood googlemail com>
Date:   Mon Sep 20 18:35:01 2021 +0100

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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]