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
Please register or sign in to comment