From 2ea5153e1c979760c56bd81f3d47f2fb373130c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Thu, 15 Feb 2018 18:04:12 +0100 Subject: [PATCH] Don't crash if the cursor theme fails to create Summary: If the cursor theme failed to create KWin crashed due to an endless recursion. There are two reasons for this fault: 1) When the physical size does not exist we perform a division by 0 which results in an invalid size going into wl_cursor_theme_load 2) We emit the signal that the cursor theme changed even if it didn't change thus creating an endless recursion This change addresses both problems: it checks that the size is not 0 and changes the handling for theme update to only destroy the previous theme if the new theme could be created and only emits the signal if things change. BUG: 390314 FIXED-IN: 5.12.3 Test Plan: Added a new test case which crashed with old code Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #plasma Differential Revision: https://phabricator.kde.org/D10549 --- autotests/integration/CMakeLists.txt | 1 + .../dont_crash_cursor_physical_size_empty.cpp | 124 ++++++++++++++++++ wayland_cursor_theme.cpp | 26 ++-- 3 files changed, 142 insertions(+), 9 deletions(-) create mode 100644 autotests/integration/dont_crash_cursor_physical_size_empty.cpp diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index bb3cbb3b8d..4a89201cb2 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -55,6 +55,7 @@ integrationTest(WAYLAND_ONLY NAME testVirtualDesktop SRCS virtual_desktop_test.c integrationTest(WAYLAND_ONLY NAME testShellClientRules SRCS shell_client_rules_test.cpp) integrationTest(WAYLAND_ONLY NAME testIdleInhibition SRCS idle_inhibition_test.cpp) integrationTest(WAYLAND_ONLY NAME testColorCorrectNightColor SRCS colorcorrect_nightcolor_test.cpp) +integrationTest(WAYLAND_ONLY NAME testDontCrashCursorPhysicalSizeEmpty SRCS dont_crash_cursor_physical_size_empty.cpp) if (XCB_ICCCM_FOUND) integrationTest(NAME testMoveResize SRCS move_resize_window_test.cpp LIBS XCB::ICCCM) diff --git a/autotests/integration/dont_crash_cursor_physical_size_empty.cpp b/autotests/integration/dont_crash_cursor_physical_size_empty.cpp new file mode 100644 index 0000000000..b6e5a3cb52 --- /dev/null +++ b/autotests/integration/dont_crash_cursor_physical_size_empty.cpp @@ -0,0 +1,124 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2018 Martin Flöser + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "kwin_wayland_test.h" +#include "composite.h" +#include "effectloader.h" +#include "client.h" +#include "cursor.h" +#include "effects.h" +#include "platform.h" +#include "shell_client.h" +#include "screens.h" +#include "wayland_server.h" +#include "workspace.h" + +#include + +#include +#include +#include +#include +#include +#include + +using namespace KWin; +using namespace KWayland::Client; +static const QString s_socketName = QStringLiteral("wayland_test_kwin_crash_cursor_physical_size_empty-0"); + +class DontCrashCursorPhysicalSizeEmpty : public QObject +{ +Q_OBJECT +private Q_SLOTS: + void init(); + void initTestCase(); + void cleanup(); + void testMoveCursorOverDeco_data(); + void testMoveCursorOverDeco(); +}; + +void DontCrashCursorPhysicalSizeEmpty::init() +{ + QVERIFY(Test::setupWaylandConnection(Test::AdditionalWaylandInterface::Decoration)); + + screens()->setCurrent(0); + KWin::Cursor::setPos(QPoint(640, 512)); +} + +void DontCrashCursorPhysicalSizeEmpty::cleanup() +{ + Test::destroyWaylandConnection(); +} + +void DontCrashCursorPhysicalSizeEmpty::initTestCase() +{ + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024)); + QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit())); + + if (!QStandardPaths::locateAll(QStandardPaths::GenericDataLocation, QStringLiteral("icons/DMZ-White/index.theme")).isEmpty()) { + qputenv("XCURSOR_THEME", QByteArrayLiteral("DMZ-White")); + } else { + // might be vanilla-dmz (e.g. Arch, FreeBSD) + qputenv("XCURSOR_THEME", QByteArrayLiteral("Vanilla-DMZ")); + } + qputenv("XCURSOR_SIZE", QByteArrayLiteral("0")); + + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); +} + +void DontCrashCursorPhysicalSizeEmpty::testMoveCursorOverDeco_data() +{ + QTest::addColumn("type"); + + QTest::newRow("wlShell") << Test::ShellSurfaceType::WlShell; + QTest::newRow("xdgShellV5") << Test::ShellSurfaceType::XdgShellV5; + QTest::newRow("xdgShellV6") << Test::ShellSurfaceType::XdgShellV6; +} + +void DontCrashCursorPhysicalSizeEmpty::testMoveCursorOverDeco() +{ + // This test ensures that there is no endless recursion if the cursor theme cannot be created + // a reason for creation failure could be physical size not existing + // see BUG: 390314 + QScopedPointer surface(Test::createSurface()); + QFETCH(Test::ShellSurfaceType, type); + Test::waylandServerSideDecoration()->create(surface.data(), surface.data()); + QScopedPointer shellSurface(Test::createShellSurface(type, surface.data())); + + auto c = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); + QVERIFY(c); + QVERIFY(c->isDecorated()); + + // destroy physical size + KWayland::Server::Display *display = waylandServer()->display(); + auto output = display->outputs().first(); + output->setPhysicalSize(QSize(0, 0)); + // and fake a cursor theme change, so that the theme gets recreated + emit KWin::Cursor::self()->themeChanged(); + + KWin::Cursor::setPos(QPoint(c->geometry().center().x(), c->clientPos().y() / 2)); +} + +WAYLANDTEST_MAIN(DontCrashCursorPhysicalSizeEmpty) +#include "dont_crash_cursor_physical_size_empty.moc" diff --git a/wayland_cursor_theme.cpp b/wayland_cursor_theme.cpp index 682a5faa96..42b3bc445b 100644 --- a/wayland_cursor_theme.cpp +++ b/wayland_cursor_theme.cpp @@ -50,12 +50,6 @@ void WaylandCursorTheme::loadTheme() return; } Cursor *c = Cursor::self(); - if (!m_theme) { - // so far the theme had not been created, this means we need to start tracking theme changes - connect(c, &Cursor::themeChanged, this, &WaylandCursorTheme::loadTheme); - } else { - destroyTheme(); - } int size = c->themeSize(); if (size == 0) { // resolution depended @@ -63,11 +57,25 @@ void WaylandCursorTheme::loadTheme() KWayland::Server::Display *display = waylandServer()->display(); auto output = display->outputs().first(); // calculate dots per inch, multiplied with magic constants from Cursor::loadThemeSettings() - size = qreal(output->pixelSize().height()) / (qreal(output->physicalSize().height()) * 0.0393701) * 16.0 / 72.0; + if (output->physicalSize().height()) { + size = qreal(output->pixelSize().height()) / (qreal(output->physicalSize().height()) * 0.0393701) * 16.0 / 72.0; + } else { + // use sensible default + size = 24; + } } - m_theme = wl_cursor_theme_load(c->themeName().toUtf8().constData(), + auto theme = wl_cursor_theme_load(c->themeName().toUtf8().constData(), size, m_shm->shm()); - emit themeChanged(); + if (theme) { + if (!m_theme) { + // so far the theme had not been created, this means we need to start tracking theme changes + connect(c, &Cursor::themeChanged, this, &WaylandCursorTheme::loadTheme); + } else { + destroyTheme(); + } + m_theme = theme; + emit themeChanged(); + } } void WaylandCursorTheme::destroyTheme()