Fix heap-use-after-free with QQuickListView

Ensure we stop FXViewItems from tracking so we don't have pointers
to dead QQuickListViewPrivate object on QQuickItem destruction.

Fixes: QTBUG-71581
Change-Id: I80291086697b1455d9319969fe5cba0ea4d04a73
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
 

4 files changed, 26 insertions, 19 deletions

diff --git a/src/quick/items/qquickgridview.cpp b/src/quick/items/qquickgridview.cpp
index 6638fbd3e8..fecfa5fd2d 100644
--- a/src/quick/items/qquickgridview.cpp
+++ b/src/quick/items/qquickgridview.cpp
@@ -191,7 +191,7 @@ public:
     void resetFirstItemPosition(qreal pos = 0.0) override;
     void adjustFirstItem(qreal forwards, qreal backwards, int changeBeforeVisible) override;
 
-    void createHighlight() override;
+    void createHighlight(bool onDestruction = false) override;
     void updateHighlight() override;
     void resetHighlightPosition() override;
 
@@ -696,9 +696,8 @@ void QQuickGridViewPrivate::adjustFirstItem(qreal forwards, qreal backwards, int
     gridItem->setPosition(gridItem->colPos(), gridItem->rowPos() + ((moveCount / columns) * rowSize()));
 }
 
-void QQuickGridViewPrivate::createHighlight()
+void QQuickGridViewPrivate::createHighlight(bool onDestruction)
 {
-    Q_Q(QQuickGridView);
     bool changed = false;
     if (highlight) {
         if (trackedItem == highlight)
@@ -714,6 +713,10 @@ void QQuickGridViewPrivate::createHighlight()
         changed = true;
     }
 
+    if (onDestruction)
+        return;
+
+    Q_Q(QQuickGridView);
     if (currentItem) {
         QQuickItem *item = createHighlightItem();
         if (item) {
diff --git a/src/quick/items/qquickitemview.cpp b/src/quick/items/qquickitemview.cpp
index 1f8a0de72b..431cb80a75 100644
--- a/src/quick/items/qquickitemview.cpp
+++ b/src/quick/items/qquickitemview.cpp
@@ -162,7 +162,7 @@ QQuickItemView::QQuickItemView(QQuickFlickablePrivate &dd, QQuickItem *parent)
 QQuickItemView::~QQuickItemView()
 {
     Q_D(QQuickItemView);
-    d->clear();
+    d->clear(true);
     if (d->ownModel)
         delete d->model;
     delete d->header;
@@ -1662,7 +1662,7 @@ void QQuickItemViewPrivate::updateCurrent(int modelIndex)
     releaseItem(oldCurrentItem);
 }
 
-void QQuickItemViewPrivate::clear()
+void QQuickItemViewPrivate::clear(bool onDestruction)
 {
     Q_Q(QQuickItemView);
     currentChanges.reset();
@@ -1683,7 +1683,7 @@ void QQuickItemViewPrivate::clear()
     currentItem = nullptr;
     if (oldCurrentItem)
         emit q->currentItemChanged();
-    createHighlight();
+    createHighlight(onDestruction);
     trackedItem = nullptr;
 
     if (requestedIndex >= 0) {
@@ -2352,15 +2352,16 @@ void QQuickItemView::destroyingItem(QObject *object)
 bool QQuickItemViewPrivate::releaseItem(FxViewItem *item)
 {
     Q_Q(QQuickItemView);
-    if (!item || !model)
+    if (!item)
         return true;
     if (trackedItem == item)
         trackedItem = nullptr;
     item->trackGeometry(false);
 
-    QQmlInstanceModel::ReleaseFlags flags = model->release(item->item);
-    if (item->item) {
-        if (flags == 0) {
+    QQmlInstanceModel::ReleaseFlags flags = {};
+    if (model && item->item) {
+        flags = model->release(item->item);
+        if (!flags) {
             // item was not destroyed, and we no longer reference it.
             QQuickItemPrivate::get(item->item)->setCulled(true);
             unrequestedItems.insert(item->item, model->indexOf(item->item, q));
diff --git a/src/quick/items/qquickitemview_p_p.h b/src/quick/items/qquickitemview_p_p.h
index ea5b5df9c6..170027de87 100644
--- a/src/quick/items/qquickitemview_p_p.h
+++ b/src/quick/items/qquickitemview_p_p.h
@@ -163,7 +163,7 @@ public:
     int mapFromModel(int modelIndex) const;
 
     virtual void init();
-    virtual void clear();
+    virtual void clear(bool onDestruction=false);
     virtual void updateViewport();
 
     void regenerate(bool orientationChanged=false);
@@ -327,7 +327,7 @@ protected:
     virtual bool hasStickyHeader() const { return false; }
     virtual bool hasStickyFooter() const { return false; }
 
-    virtual void createHighlight() = 0;
+    virtual void createHighlight(bool onDestruction = false) = 0;
     virtual void updateHighlight() = 0;
     virtual void resetHighlightPosition() = 0;
     virtual bool movingFromHighlight() { return false; }
diff --git a/src/quick/items/qquicklistview.cpp b/src/quick/items/qquicklistview.cpp
index 81d019a26d..146917c679 100644
--- a/src/quick/items/qquicklistview.cpp
+++ b/src/quick/items/qquicklistview.cpp
@@ -81,7 +81,7 @@ public:
     FxViewItem *snapItemAt(qreal pos);
 
     void init() override;
-    void clear() override;
+    void clear(bool onDestruction) override;
 
     bool addVisibleItems(qreal fillFrom, qreal fillTo, qreal bufferFrom, qreal bufferTo, bool doBuffer) override;
     bool removeNonVisibleItems(qreal bufferFrom, qreal bufferTo) override;
@@ -98,7 +98,7 @@ public:
     void adjustFirstItem(qreal forwards, qreal backwards, int) override;
     void updateSizeChangesBeforeVisiblePos(FxViewItem *item, ChangeResult *removeResult) override;
 
-    void createHighlight() override;
+    void createHighlight(bool onDestruction = false) override;
     void updateHighlight() override;
     void resetHighlightPosition() override;
     bool movingFromHighlight() override;
@@ -575,7 +575,7 @@ void QQuickListViewPrivate::init()
     ::memset(sectionCache, 0, sizeof(QQuickItem*) * sectionCacheSize);
 }
 
-void QQuickListViewPrivate::clear()
+void QQuickListViewPrivate::clear(bool onDestruction)
 {
     for (int i = 0; i < sectionCacheSize; ++i) {
         delete sectionCache[i];
@@ -587,7 +587,7 @@ void QQuickListViewPrivate::clear()
     releaseSectionItem(nextSectionItem);
     nextSectionItem = nullptr;
     lastVisibleSection = QString();
-    QQuickItemViewPrivate::clear();
+    QQuickItemViewPrivate::clear(onDestruction);
 }
 
 FxViewItem *QQuickListViewPrivate::newViewItem(int modelIndex, QQuickItem *item)
@@ -634,7 +634,7 @@ void QQuickListViewPrivate::initializeViewItem(FxViewItem *item)
 bool QQuickListViewPrivate::releaseItem(FxViewItem *item)
 {
     if (!item || !model)
-        return true;
+        return QQuickItemViewPrivate::releaseItem(item);
 
     QPointer<QQuickItem> it = item->item;
     QQuickListViewAttached *att = static_cast<QQuickListViewAttached*>(item->attached);
@@ -876,9 +876,8 @@ void QQuickListViewPrivate::updateSizeChangesBeforeVisiblePos(FxViewItem *item,
         QQuickItemViewPrivate::updateSizeChangesBeforeVisiblePos(item, removeResult);
 }
 
-void QQuickListViewPrivate::createHighlight()
+void QQuickListViewPrivate::createHighlight(bool onDestruction)
 {
-    Q_Q(QQuickListView);
     bool changed = false;
     if (highlight) {
         if (trackedItem == highlight)
@@ -896,6 +895,10 @@ void QQuickListViewPrivate::createHighlight()
         changed = true;
     }
 
+    if (onDestruction)
+        return;
+
+    Q_Q(QQuickListView);
     if (currentItem) {
         QQuickItem *item = createHighlightItem();
         if (item) {