[plasma/libksysguard] /: systemstats: don't keep dangling SensorObject pointers in SensorContainer

Git commit e72f83052f8a8191239831125c75bd0e59d004c8 by Méven Car.
Committed on 27/07/2026 at 17:43.
Pushed by ngraham into branch 'master'.

systemstats: don't keep dangling SensorObject pointers in SensorContainer

A SensorObject was only removed from its SensorContainer when it emitted
aboutToBeRemoved; the destructor did neither that nor any deregistration. An
object deleted by any other route (directly, or by its QObject parent) therefore
left a dangling raw pointer in the container. When another object was later added,
objectAdded triggered AggregateSensor::updateSensors(), which iterates objects()
and calls id() on each, dereferencing the freed object and crashing in the
QString copy.

Connect each object's QObject::destroyed to drop it from the container, comparing
by identity so re-adding an object with the same id is unaffected. Also register
objects with the object itself as the queued-call context, so an object destroyed
before its queued addObject() runs no longer calls addObject() on a dangling
pointer.

BUG: 523562

M  +1    -0    autotests/CMakeLists.txt
A  +85   -0    autotests/sensorcontainertest.cpp     [License: LGPL(v2.0+)]
M  +6    -0    systemstats/SensorContainer.cpp
M  +1    -1    systemstats/SensorObject.cpp

https://invent.kde.org/plasma/libksysguard/-/commit/e72f83052f8a8191239831125c75bd0e59d004c8

diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
index b03c896a..16a3e4bf 100644
--- a/autotests/CMakeLists.txt
+++ b/autotests/CMakeLists.txt
@@ -10,6 +10,7 @@ ecm_add_test(formattertest.cpp LINK_LIBRARIES Qt::Test KSysGuard::Formatter)
 ecm_add_test(sensortreemodeltest.cpp LINK_LIBRARIES Qt::Test Qt::DBus KSysGuard::Sensors)
 ecm_add_test(sensordatamodeltest.cpp LINK_LIBRARIES Qt::Test Qt::DBus KSysGuard::Sensors KSysGuard::Formatter)
 ecm_add_test(processdatamodeltest.cpp LINK_LIBRARIES Qt::Test KSysGuard::ProcessCore)
+ecm_add_test(sensorcontainertest.cpp LINK_LIBRARIES Qt::Test KSysGuard::SystemStats)
 ecm_add_test(cgrouptest.cpp LINK_LIBRARIES Qt::Test KF6::Service KSysGuard::ProcessCore)
 
 include_directories(../faces)
diff --git a/autotests/sensorcontainertest.cpp b/autotests/sensorcontainertest.cpp
new file mode 100644
index 00000000..5ffd3798
--- /dev/null
+++ b/autotests/sensorcontainertest.cpp
@@ -0,0 +1,85 @@
+/*
+    SPDX-FileCopyrightText: 2026 Méven Car <[email protected]>
+
+    SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include <QTest>
+
+#include "systemstats/SensorContainer.h"
+#include "systemstats/SensorObject.h"
+#include "systemstats/SensorPlugin.h"
+
+using namespace KSysGuard;
+
+class SensorContainerTest : public QObject
+{
+    Q_OBJECT
+private Q_SLOTS:
+    void deletedObjectIsRemovedFromContainer();
+    void destroyedHandlerDoesNotEvictReplacement();
+    void objectDeletedBeforeQueuedRegistration();
+};
+
+// A SensorObject deleted without going through aboutToBeRemoved/removeObject (as
+// happens when a plugin drops a device) must not leave a dangling pointer in its
+// container, or iterating objects() and reading their ids crashes (bug 523562).
+void SensorContainerTest::deletedObjectIsRemovedFromContainer()
+{
+    SensorPlugin plugin(nullptr, {});
+    auto container = new SensorContainer(QStringLiteral("test"), QStringLiteral("Test"), &plugin);
+
+    auto object = new SensorObject(QStringLiteral("obj"), container);
+    // Registration with the container is queued from the constructor.
+    QTRY_COMPARE(container->objects().size(), 1);
+
+    delete object;
+
+    QCOMPARE(container->objects().size(), 0);
+    // The exact operation AggregateSensor::updateSensors() performs; a use-after-free
+    // before the fix.
+    for (auto obj : container->objects()) {
+        obj->id();
+    }
+}
+
+// The destroyed() cleanup keys on the object id, so it must compare identity and not
+// evict a replacement object that reused the same id.
+void SensorContainerTest::destroyedHandlerDoesNotEvictReplacement()
+{
+    SensorPlugin plugin(nullptr, {});
+    auto container = new SensorContainer(QStringLiteral("test"), QStringLiteral("Test"), &plugin);
+
+    auto first = new SensorObject(QStringLiteral("dup"), container);
+    QTRY_COMPARE(container->objects().size(), 1);
+
+    // Detach the first object (still alive), then register a replacement with the same id.
+    container->removeObject(first);
+    QCOMPARE(container->objects().size(), 0);
+
+    auto second = new SensorObject(QStringLiteral("dup"), container);
+    QTRY_COMPARE(container->object(QStringLiteral("dup")), second);
+
+    // Destroying the detached first object must not evict the replacement.
+    delete first;
+    QCOMPARE(container->object(QStringLiteral("dup")), second);
+}
+
+// A SensorObject deleted before its queued registration runs must not cause
+// addObject() to be called on a dangling pointer.
+void SensorContainerTest::objectDeletedBeforeQueuedRegistration()
+{
+    SensorPlugin plugin(nullptr, {});
+    auto container = new SensorContainer(QStringLiteral("test"), QStringLiteral("Test"), &plugin);
+
+    auto object = new SensorObject(QStringLiteral("ephemeral"), container);
+    delete object;
+
+    // Drain the event loop; the queued addObject() must have been discarded.
+    QTest::qWait(50);
+    QCOMPARE(container->objects().size(), 0);
+}
+
+QTEST_GUILESS_MAIN(SensorContainerTest)
+
+#include "sensorcontainertest.moc"
diff --git a/systemstats/SensorContainer.cpp b/systemstats/SensorContainer.cpp
index d16a0aff..28c811b4 100644
--- a/systemstats/SensorContainer.cpp
+++ b/systemstats/SensorContainer.cpp
@@ -68,6 +68,12 @@ void SensorContainer::addObject(SensorObject *object)
     connect(object, &SensorObject::aboutToBeRemoved, this, [this, object]() {
         removeObject(object);
     });
+
+    connect(object, &QObject::destroyed, this, [this, objectId](QObject *destroyedObject) {
+        if (d->sensorObjects.value(objectId) == destroyedObject) {
+            d->sensorObjects.remove(objectId);
+        }
+    });
 }
 
 void SensorContainer::removeObject(SensorObject *object)
diff --git a/systemstats/SensorObject.cpp b/systemstats/SensorObject.cpp
index 8e6b4360..7109d5b1 100644
--- a/systemstats/SensorObject.cpp
+++ b/systemstats/SensorObject.cpp
@@ -36,7 +36,7 @@ SensorObject::SensorObject(const QString &id, const QString &name, SensorContain
 
     if (parent) {
         QMetaObject::invokeMethod(
-            parent,
+            this,
             [this, parent] {
                 parent->addObject(this);
             },