Skip to content
Commit 4cc426c6 authored by Mike Fleetwood's avatar Mike Fleetwood Committed by Curtis Gedak
Browse files

Only allow Undo and Apply after merging operations (#699452)

It was possible to make GParted crash by adding a label, check or new
UUID operation and then applying the operation before the view of
pending operations had finished fully opening.  The operation would be
successfully applied but GParted would crash afterwards.

The fault was that Add_Operation() still enabled the Undo and Apply
buttons and processed the GTK event loop before merging the list of
pending operations.  Faulty code flow went like this:

    activate_*()
        Add_Operation()
            Add operation to the operations[] vector
            Enable Undo and Apply buttons
            Refresh_Visual()
                Process GTK event loop
                    Process Apply button callback applying operations,
                    refreshing display and clearing operations[] vector
        Merge operations in the operations[] vector
        << Core dump here >>
            Merge_Operations()
                Refresh_Visual()

This faulty code flow came about when merging of operations was added
and it didn't appreciate that the operations[] vector could have been
processed and cleared by Add_Operations() before the merge step.
Relevant commit:

    b10349ae
    Merge overlapping operations (#438573)

Fragment of code in the label operation case:

  2454  void Win_GParted::activate_label_partition()
  2455  {
  ...
  2472          Add_Operation( operation ) ;
  2473
  2474          // Verify if the two operations can be merged
  2475          for ( unsigned int t = 0 ; t < operations .size() - 1 ; t++ )
  2476          {
  2477              if ( operations[ t ] ->type == OPERATION_LABEL_PARTITION )
  2478              {
  2479                  if ( Merge_Operations( t, operations .size() - 1 ) )
  2480                      break;
  2481              }
  2482          }

Commentary in the crashing label operation case:

  2472  The pending operation was already applied when Add_Operation()
        returned resulting in the operations[] vector being cleared
        setting its size to 0.
  2475  The return type of operations.size() is an unsigned integral, so
        the upper limit of the for loop is t < 0UL - 1.  Assuming a
        32-bit machine that's t < 4294967295.
  2477  operations[] vector is access from out of bounds offset 0
        upwards until unallocated memory is accessed resulting in a core
        dump.

Fix this by not enabling the Undo and Apply buttons and processing the
GTK event loop until after merging of operations has been performed.
Fixed code flow goes like this:

    activate_*()
        Add_Operation()
            Add operation to the operations[] vector
        Merge operations in the operations[] vector
            Merge_Operations()
        show_operationslist()
            Enable Undo and Apply buttons
            Refresh_Visual()
                Process GTK event loop
                    Process Apply button callback applying operations,
                    refreshing display and clearing operations[] vector

Not allowing the operations list to be process until after the merge
step is the be correct ordering.  This also prevents the new operation
from flashing up in the operations list and then immediately
disappearing if merged.  In the case of adding the first operation,
delaying enabling the Undo and Apply buttons is enough as the buttons
were previously disabled preventing the operation being applied before
the merge.  In the case of adding further operations, processing of the
GTK event loop must also be delayed until after the merge to prevent the
operations being applied before the merge.  Although that window of
opportunity would only be microseconds.

Bug #699452 - Crash when applying operations before pending operations
              fully displayed
parent d9a3f879
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment