From 3f1db0cfde4db6cdbb10a9831a09cf82892a5c43 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Mon, 30 May 2016 09:53:53 +0200
Subject: [PATCH] [server] Don't destroy BlurInterface when parent
 SurfaceInterface is destroyed

Summary:
Destroying the BlurInterface on the server side before the client has a
chance to cleanup results in a protocol error:
wl_display@1: error 0: invalid object 7

Which would terminate the client. If we would not destroy the resource,
but only delete the BlurInterface it could result in heap-use-after-free.

So just don't do anything, the client needs to cleanup which will result
in the BlurInterface being deleted.

Reviewers: #plasma

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D1708
---
 .../autotests/client/test_wayland_blur.cpp    | 34 +++++++++++++++++++
 src/wayland/blur_interface.cpp                |  8 -----
 2 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/src/wayland/autotests/client/test_wayland_blur.cpp b/src/wayland/autotests/client/test_wayland_blur.cpp
index fa93ada582..cad680d28f 100644
--- a/src/wayland/autotests/client/test_wayland_blur.cpp
+++ b/src/wayland/autotests/client/test_wayland_blur.cpp
@@ -44,6 +44,7 @@ private Q_SLOTS:
     void cleanup();
 
     void testCreate();
+    void testSurfaceDestroy();
 
 private:
     KWayland::Server::Display *m_display;
@@ -177,5 +178,38 @@ void TestBlur::testCreate()
     QVERIFY(destroyedSpy.wait());
 }
 
+void TestBlur::testSurfaceDestroy()
+{
+    QSignalSpy serverSurfaceCreated(m_compositorInterface, &KWayland::Server::CompositorInterface::surfaceCreated);
+    QVERIFY(serverSurfaceCreated.isValid());
+
+    QScopedPointer<KWayland::Client::Surface> surface(m_compositor->createSurface());
+    QVERIFY(serverSurfaceCreated.wait());
+
+    auto serverSurface = serverSurfaceCreated.first().first().value<KWayland::Server::SurfaceInterface*>();
+    QSignalSpy blurChanged(serverSurface, &KWayland::Server::SurfaceInterface::blurChanged);
+    QVERIFY(blurChanged.isValid());
+
+    QScopedPointer<KWayland::Client::Blur> blur(m_blurManager->createBlur(surface.data()));
+    blur->setRegion(m_compositor->createRegion(QRegion(0, 0, 10, 20), nullptr));
+    blur->commit();
+    surface->commit(KWayland::Client::Surface::CommitFlag::None);
+
+    QVERIFY(blurChanged.wait());
+    QCOMPARE(serverSurface->blur()->region(), QRegion(0, 0, 10, 20));
+
+    // destroy the parent surface
+    QSignalSpy surfaceDestroyedSpy(serverSurface, &QObject::destroyed);
+    QVERIFY(surfaceDestroyedSpy.isValid());
+    QSignalSpy blurDestroyedSpy(serverSurface->blur(), &QObject::destroyed);
+    QVERIFY(blurDestroyedSpy.isValid());
+    surface.reset();
+    QVERIFY(surfaceDestroyedSpy.wait());
+    QVERIFY(blurDestroyedSpy.isEmpty());
+    // destroy the blur
+    blur.reset();
+    QVERIFY(blurDestroyedSpy.wait());
+}
+
 QTEST_GUILESS_MAIN(TestBlur)
 #include "test_wayland_blur.moc"
diff --git a/src/wayland/blur_interface.cpp b/src/wayland/blur_interface.cpp
index 258c49a4d6..f0de13fd13 100644
--- a/src/wayland/blur_interface.cpp
+++ b/src/wayland/blur_interface.cpp
@@ -106,14 +106,6 @@ void BlurManagerInterface::Private::createBlur(wl_client *client, wl_resource *r
         delete blur;
         return;
     }
-    QObject::connect(s, &QObject::destroyed, blur,
-        [blur] {
-            if (blur->resource()) {
-                wl_resource_destroy(blur->resource());
-                delete blur;
-            }
-        }
-    );
     s->d_func()->setBlur(QPointer<BlurInterface>(blur));
 }