QQuickPixmapReader::asyncResponseFinished segfaults if a QQuickAsyncImageProvider returns an error response

Bug #1469611 reported by James Henstridge
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
qtdeclarative-opensource-src (Ubuntu)
Fix Released
High
Timo Jyrinki
qtdeclarative-opensource-src (Ubuntu RTM)
Fix Released
High
Timo Jyrinki

Bug Description

While working on the new thumbnailer, we've been using the new QQuickAsyncImageProvider API. The API allows us to report errors by overriding the errorString() method on QQuickImageResponse to return a non-empty string. However, if I do so the application crashes.

Loading up the symbols to get a stack trace shows this to be a bug in the logic of QQuickPixmapReader::asyncResponseFinished:

        QQuickTextureFactory *t = 0;
        QQuickPixmapReply::ReadError error = QQuickPixmapReply::NoError;
        QString errorString;
        QSize readSize;
        if (!response->errorString().isEmpty()) {
            error = QQuickPixmapReply::Loading;
            errorString = response->errorString();
        } else {
            t = response->textureFactory();
       }
        mutex.lock();
        if (!cancelled.contains(job))
            job->postReply(error, errorString, t->textureSize(), t);
        mutex.unlock();

If errorString() is not empty, then t will still be NULL. It is then dereferenced to call t->textureSize() resulting in a segfault.

ProblemType: Bug
DistroRelease: Ubuntu 15.10
Package: libqt5quick5 5.4.2-1ubuntu1
ProcVersionSignature: Ubuntu 3.19.0-20.20-generic 3.19.8
Uname: Linux 3.19.0-20-generic x86_64
ApportVersion: 2.17.3-0ubuntu4
Architecture: amd64
CurrentDesktop: Unity
Date: Mon Jun 29 14:53:49 2015
InstallationDate: Installed on 2013-10-29 (607 days ago)
InstallationMedia: Ubuntu 13.10 "Saucy Salamander" - Release amd64 (20131016.1)
SourcePackage: qtdeclarative-opensource-src
UpgradeStatus: Upgraded to wily on 2015-06-13 (15 days ago)

Test case (on desktop):

- bzr branch lp:~jamesh/thumbnailer/no-fallback-albumart
- sudo apt-get build-dep thumbnailer
- sudo apt install libleveldb-dev cmake-extras libapparmor-dev libboost-filesystem-dev libboost-regex-dev libqtdbustest1-dev libunity-api-dev python3-tornado qml-module-qttest xvfb
- cd no-fallback-albumart
- cmake .
- make
- ctest -R qml --verbose
-> check if crash (Segmentation fault (core dumped)) or no crash (don't mind tests pass/fail)

Revision history for this message
James Henstridge (jamesh) wrote :
Revision history for this message
Albert Astals Cid (aacid) wrote :

Fixed by https://codereview.qt-project.org/#/c/115522/

Let's wait for upstream before distro-patching i guess

Revision history for this message
Albert Astals Cid (aacid) wrote :

It's been now merged upstream.

Timo can you please take care of merging it to our distro-patch?

Changed in qtdeclarative-opensource-src (Ubuntu):
importance: Undecided → High
assignee: nobody → Timo Jyrinki (timo-jyrinki)
status: New → Triaged
Changed in qtdeclarative-opensource-src (Ubuntu):
status: Triaged → In Progress
Changed in qtdeclarative-opensource-src (Ubuntu RTM):
assignee: nobody → Timo Jyrinki (timo-jyrinki)
Revision history for this message
Albert Astals Cid (aacid) wrote :

Timo can you please also include this one? https://codereview.qt-project.org/#/c/120638/

Changed in qtdeclarative-opensource-src (Ubuntu RTM):
importance: Undecided → High
status: New → In Progress
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

@Albert including that too.

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

Please test landing-043 silo with vivid-proposed and report back together with which device + image number it was tested with.

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

s/vivid-proposed/vivid-overlay/

Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

< jamesh> Mirv: okay. The tests in my thumbnailer branch no longer crash with the silo applied.

description: updated
description: updated
description: updated
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package qtdeclarative-opensource-src - 5.4.2-1ubuntu2

---------------
qtdeclarative-opensource-src (5.4.2-1ubuntu2) wily; urgency=medium

  * debian/patches/Add-QQuickAsyncImageProvider.patch:
    - Fix segfault by importing https://codereview.qt-project.org/#/c/115522/
      and https://codereview.qt-project.org/#/c/120638/
      (LP: #1469611)

 -- Timo Jyrinki <email address hidden> Wed, 08 Jul 2015 05:20:41 +0000

Changed in qtdeclarative-opensource-src (Ubuntu):
status: In Progress → Fix Released
Revision history for this message
Timo Jyrinki (timo-jyrinki) wrote :

qtdeclarative-opensource-src (5.4.1-1ubuntu8) vivid; urgency=medium

  * debian/patches/Add-QQuickAsyncImageProvider.patch:
    - Fix segfault by importing https://codereview.qt-project.org/#/c/115522/
      and https://codereview.qt-project.org/#/c/120638/
      (LP: #1469611)

 -- Timo Jyrinki <email address hidden> Tue, 07 Jul 2015 11:50:57 +0000

Changed in qtdeclarative-opensource-src (Ubuntu RTM):
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers