From 1aa4eab4a68e19702b5b3ab9b831efdc35266e66 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Mon, 13 May 2019 14:19:01 +0200 Subject: 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 --- src/quick/items/qquickgridview.cpp | 9 ++++++--- src/quick/items/qquickitemview.cpp | 15 ++++++++------- src/quick/items/qquickitemview_p_p.h | 4 ++-- src/quick/items/qquicklistview.cpp | 17 ++++++++++------- 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 it = item->item; QQuickListViewAttached *att = static_cast(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) { -- cgit v1.2.3