From b87937a20e80c0c296977bbe348de8e461e2c023 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 7 Oct 2023 15:35:15 +0200 Subject: [PATCH] Fix crash when double-clicking on TempoMap Ruler markers heap-use-after-free. Marker is deleted (and re-created), when the tempo-map edit is aborted: ``` #0 0x7f77528ac017 in operator delete(void*) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:160 #1 0x55f81062800d in TempoMarker::~TempoMarker() ../gtk2_ardour/marker.cc:794 #2 0x55f80ffb2fc0 in Editor::reset_tempo_marks() ../gtk2_ardour/editor_tempodisplay.cc:205 #3 0x55f80ffb2b19 in Editor::reset_metric_marks() ../gtk2_ardour/editor_tempodisplay.cc:185 #4 0x55f80ffb49fb in Editor::tempo_map_changed() ../gtk2_ardour/editor_tempodisplay.cc:301 #5 0x55f80ffbdf00 in Editor::abort_tempo_map_edit() ../gtk2_ardour/editor_tempodisplay.cc:850 #6 0x55f80fcf967a in TempoMarkerDrag::finished(_GdkEvent*, bool) ../gtk2_ardour/editor_drag.cc:333 ``` Since no movement occurred, the tempo-map was not changed. however we need to drop the lock and writable thread-pointer... --- gtk2_ardour/editor_drag.cc | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/gtk2_ardour/editor_drag.cc b/gtk2_ardour/editor_drag.cc index d90e4671ff..5cbf8ec273 100644 --- a/gtk2_ardour/editor_drag.cc +++ b/gtk2_ardour/editor_drag.cc @@ -3133,10 +3133,13 @@ void MeterMarkerDrag::finished (GdkEvent* event, bool movement_occurred) { if (!movement_occurred) { + /* get reference before _marker is deleted via reset_meter_marks due to abort_tempo_map_edit */ + Temporal::MeterPoint& section (const_cast(_marker->meter ())); /* reset thread local tempo map to the original state */ _editor->abort_tempo_map_edit (); + if (was_double_click ()) { - _editor->edit_meter_marker (*_marker); + _editor->edit_meter_section (section); } return; } @@ -3226,7 +3229,8 @@ TempoCurveDrag::finished (GdkEvent* event, bool movement_occurred) _editor->abort_tempo_map_edit (); if (was_double_click ()) { - // XXX would be nice to do this + // XXX would be nice to do this, + // but note that ::abort_tempo_map_edit() will have deleted _marker // _editor->edit_tempo_marker (*_marker); } @@ -3320,14 +3324,13 @@ TempoMarkerDrag::finished (GdkEvent* event, bool movement_occurred) { if (!movement_occurred) { - /* reset the per-thread tempo map ptr back to the current - * official version - */ - + /* get reference before _marker is deleted by reset_tempo_marks due to abort_tempo_map_edit */ + Temporal::TempoPoint& section (const_cast(_marker->tempo ())); + /* reset thread local tempo map to the original state */ _editor->abort_tempo_map_edit (); if (was_double_click ()) { - _editor->edit_tempo_marker (*_marker); + _editor->edit_tempo_section (section); } return; @@ -3403,14 +3406,12 @@ void BBTMarkerDrag::finished (GdkEvent* event, bool movement_occurred) { if (!movement_occurred) { - /* reset the per-thread tempo map ptr back to the current - * official version - */ - + Temporal::MusicTimePoint& point (const_cast(_marker->mt_point ())); + /* reset thread local tempo map to the original state */ _editor->abort_tempo_map_edit (); if (was_double_click ()) { - _editor->edit_bbt_marker (*_marker); + _editor->edit_bbt (point); } return;