Bug 400660 - Timeline::getTracksInfo() tries to dereference null pointer
Summary: Timeline::getTracksInfo() tries to dereference null pointer
Status: RESOLVED WORKSFORME
Alias: None
Product: kdenlive
Classification: Applications
Component: User Interface (show other bugs)
Version: 18.04.3
Platform: Other Linux
: NOR crash
Target Milestone: ---
Assignee: Jean-Baptiste Mardelle
URL:
Keywords: junior-jobs
: 404046 (view as bug list)
Depends on:
Blocks:
 
Reported: 2018-11-04 12:45 UTC by Adam Spiers
Modified: 2020-10-08 04:33 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
fritzibaby: low_hanging+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Spiers 2018-11-04 12:45:49 UTC
In timeline.cpp, Timeline::getTracksInfo() calls Timeline::track() and then calls Timeline::info() on the result:

    QList<TrackInfo> Timeline::getTracksInfo()
    {
        QList<TrackInfo> tracks;
        for (int i = 0; i < tracksCount(); i++) {
            tracks << track(i)->info();
        }
        return tracks;
    }

However Timeline::track() can return nullptr:

    Track *Timeline::track(int i)
    {
        if (i < 0 || i >= m_tracks.count()) {
            return nullptr;
        }
        return m_tracks.at(i);
    }

Of course one would hope that this never happens, but Murphy's Law dictated that it happened to me:

Thread 1 "kdenlive" received signal SIGSEGV, Segmentation fault.
Mlt::Properties::get (this=this@entry=0x40, name=name@entry=0x555555a606e5 "kdenlive:track_name") at MltProperties.cpp:122
122             return mlt_properties_get( get_properties( ), name );
(gdb) bt
#0  0x00007ffff6fcc800 in Mlt::Properties::get(char const*) (this=this@entry=0x40, name=name@entry=0x555555a606e5 "kdenlive:track_name")
    at MltProperties.cpp:122
#1  0x0000555555749f7a in Track::info() (this=0x0) at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/timeline/track.cpp:604
#2  0x000055555573a900 in Timeline::getTracksInfo() (this=0x55555c427960)
    at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/timeline/timeline.cpp:710
#3  0x00005555556afcfe in TransitionSettings::updateProjectFormat() (this=0x5555573c7210)
    at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/project/transitionsettings.cpp:131
#4  0x00005555559c2282 in MainWindow::connectDocument() (this=0x55555654ce20)
    at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/mainwindow.cpp:1953
#5  0x00005555556a7b57 in ProjectManager::doOpenFile(QUrl const&, KAutoSaveFile*) (this=this@entry=0x5555568f5a80, url=
    "/home/adam/music/harmony2/negative-sc/video/intro/intro.kdenlive", stale=<optimized out>, stale@entry=0x0)
    at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/project/projectmanager.cpp:573
#6  0x00005555556ac0d6 in ProjectManager::openFile(QUrl const&) (this=0x5555568f5a80, url="/home/adam/music/harmony2/negative-sc/video/intro/intro.kdenlive") at /usr/src/debug/kdenlive-18.04.3-lp150.2.1.x86_64/src/project/projectmanager.cpp:509
#7  0x0000555555a0ed89 in ProjectManager::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (_o=0x5555568f5a80, _id=<optimized out>, _a=0x7fffffffaa70, _c=<optimized out>)

Notice the (this=0x0) in stack frame #1 which shows that Timeline::track(0) returned nullptr.

This is almost certainly due to a .kdenlive project file which got corrupted (I'm seeing warnings from https://thediveo.github.io/kdenlive-project-analyzer/kdenlive-project-analyzer.html), but right now it means that that entire project (which I have spent months working on) is completely unusable because it immediately crashes kdenlive on load.  I don't want to move to 18.08.x yet because IIRC it's still in beta and I need something relatively stable.

The naive fix would be to make Timeline::getTracksInfo() check for this situation and handle it gracefully, e.g. simply by skipping any value of i for which Timeline::track(i) returns nullptr.  However grepping for "track(i)->" in timeline.cpp shows very many other code paths susceptible to this same bug.  So rather than having to fix every single one of these, perhaps it would be safer to change Timeline::track() so that rather than returning nullptr, it emits a warning explaining which track index number was missing, and also what m_tracks.count() and tracksCount() evaluate to, to make it easier to spot discrepancies between the two.

And that gives rise to the question: why are two different values being used for iterating this list of tracks?  That seems to be the root of this problem.

SOFTWARE VERSIONS

kdenlive version: 18.04.3
KDE Plasma version: 5.12.5
Qt version: 5.9.4
Comment 1 Adam Spiers 2018-11-04 13:33:39 UTC
I managed to hand-edit the .kdenlive XML file to stop it crashing on load.  This diff shows the exact edit; hopefully it helps reveal what's going on.

@@ -9071,24 +9071,20 @@ by one octave&lt;/content>
   <transition id="transition0">
    <property name="a_track">0</property>
    <property name="b_track">1</property>
    <property name="compositing">0</property>
    <property name="distort">0</property>
    <property name="rotate_center">0</property>
    <property name="mlt_service">qtblend</property>
    <property name="always_active">1</property>
   </transition>
  </tractor>
- <playlist id="overlay_track">
-  <blank length="3332"/>
-  <entry out="92" producer="tractor0" in="0"/>
- </playlist>
  <producer id="producer1" title="Anonymous Submission" out="100" in="8">
   <property name="length">00:00:05;16</property>
   <property name="eof">continue</property>
   <property name="resource">/home/adam/video/foo/effects/effect1.mp4</property>
   <property name="audio_index">-1</property>
   <property name="video_index">0</property>
   <property name="mute_on_pause">1</property>
   <property name="mlt_service">avformat-novalidate</property>
   <property name="seekable">1</property>
   <property name="aspect_ratio">1</property>
@@ -9159,21 +9155,20 @@ by one octave&lt;/content>
  <tractor id="maintractor" title="Anonymous Submission" global_feed="1" out="20054" in="0">
   <track producer="black_track"/>
   <track producer="playlist0" hide="video"/>
   <track producer="playlist2" hide="video"/>
   <track producer="playlist3"/>
   <track producer="playlist4"/>
   <track producer="playlist1"/>
   <track producer="playlist5"/>
   <track producer="playlist6"/>
   <track producer="playlist7" hide="video"/>
-  <track producer="overlay_track" hide="audio"/>
   <transition id="transition2" out="3085" in="3079">
    <property name="a_track">7</property>
    <property name="b_track">8</property>
    <property name="factory">loader</property>
    <property name="mlt_service">luma</property>
    <property name="automatic">1</property>
    <property name="kdenlive_id">dissolve</property>
    <property name="force_track">0</property>
    <property name="reverse">1</property>
   </transition>
Comment 2 Adam Spiers 2018-11-04 14:33:27 UTC
I just managed to reproduce the corruption and saw this on STDERR, although I'm not sure exactly how I managed it :-/

org.kde.multimedia.kdenlive: Overlay track not found, something is wrong!!
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
org.kde.multimedia.kdenlive: /// ARGH, requesting nullptr track:  10  - MAX is:  10
Comment 3 Adam Spiers 2018-11-04 18:36:10 UTC
Ahah!  It happened again, and I noticed that the culprit track always has id="overlay_track".  Grepping the source revealed that this is created by this feature:

https://thediveo-e.blogspot.com/2016/03/kdenlive-ui-split-view-odds-and-ends.html

which with hindsight I realise is exactly what I was using each time it broke.  So this feature is definitely broken, at least in 18.04.3.

Looking more closely, I see that Timeline::createOverlay() has:

    Mlt::Producer *overlayTrack = m_tractor->track(trackIndex);
    overlayTrack->set("hide", 2);
    overlayTrack->set("id", "overlay_track");
    delete overlayTrack;
    m_hasOverlayTrack = true;

which directly affects the track count which Timeline::getTracksInfo() and many other methods rely on:

    int Timeline::tracksCount() const
    {
        return m_tractor->count() - m_hasOverlayTrack - m_usePreview;
    }

So if this count is off by one due to incorrect handling of the overlay track, it would perfectly explain the segfault seen in this bug.
Comment 4 emohr 2018-11-05 17:09:24 UTC
Create https://phabricator.kde.org/T9990
Comment 5 p 2019-01-15 14:46:13 UTC
I have encountered this too and agree with the analysis.

Reproduced on both the master[0] branch and 17.12.3-0ubuntu1 on Ubuntu 18.04. So it's not fixed, which seems contrary to the phabricator task which says it isn't an issue anymore?

[0] (latest commit Jan 8 @ 2aeae386b59b2de06beef9680fd8c2c813c9e3b5)
Comment 6 emohr 2019-01-15 16:31:35 UTC
Could you test with the current Kdenlive Kdenlive_Nightly_Appimage:

https://binary-factory.kde.org/job/Kdenlive_Nightly_Appimage_Build/lastSuccessfulBuild/artifact/
Comment 7 Adam Spiers 2019-02-02 16:46:56 UTC
I tried with that AppImage but unfortunately immediately ran into a crash.  Reported in bug #403867.
Comment 8 Mazin07 2019-02-07 19:57:01 UTC
*** Bug 404046 has been marked as a duplicate of this bug. ***
Comment 9 emohr 2020-09-08 16:24:49 UTC
Please try with the current Kdenlive AppImage version 20.08.0 to see if there are any packaging issues https://files.kde.org/kdenlive/release/ 

If the problem/issue doesn't occur when using the AppImage, then it's your configuration or packaging.

WARNING: Version 20.08.0 has a new project type that is not backwards compatible... so you won't be able to use older versions to open new files.
Comment 10 Bug Janitor Service 2020-09-23 04:33:12 UTC
Dear Bug Submitter,

This bug has been in NEEDSINFO status with no change for at least
15 days. Please provide the requested information as soon as
possible and set the bug status as REPORTED. Due to regular bug
tracker maintenance, if the bug is still in NEEDSINFO status with
no change in 30 days the bug will be closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

If you have already provided the requested information, please
mark the bug as REPORTED so that the KDE team knows that the bug is
ready to be confirmed.

Thank you for helping us make KDE software even better for everyone!
Comment 11 Bug Janitor Service 2020-10-08 04:33:16 UTC
This bug has been in NEEDSINFO status with no change for at least
30 days. The bug is now closed as RESOLVED > WORKSFORME
due to lack of needed information.

For more information about our bug triaging procedures please read the
wiki located here:
https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging

Thank you for helping us make KDE software even better for everyone!