From 0df09a8cbbb1caa217c8deb59700bd12f27e45b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Sun, 18 Jun 2017 21:01:30 +0200 Subject: [PATCH] Better handle cases when the xkb keymap fails to be created Summary: If the keymap cannot be created a few pointers in Xkb are null. We should make sure to not call any xkbcommon functions on those null pointers and instead use proper fallbacks. This change introduces fixes for a few usages, but it's not unlikely that there are more cases. BUG: 381210 FIXED-IN: 5.10.3 Test Plan: Autotest added for the condition of the bug, which does not crash any more. Just starting the test found a few more crash cases. Reviewers: #kwin, #plasma Subscribers: plasma-devel, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D6260 --- autotests/integration/CMakeLists.txt | 1 + .../keymap_creation_failure_test.cpp | 102 ++++++++++++++++++ xkb.cpp | 8 +- 3 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 autotests/integration/keymap_creation_failure_test.cpp diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index 5f1d379925..60000224e0 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -43,6 +43,7 @@ integrationTest(NAME testGlobalShortcuts SRCS globalshortcuts_test.cpp) integrationTest(NAME testWindowSelection SRCS window_selection_test.cpp) integrationTest(NAME testPointerConstraints SRCS pointer_constraints_test.cpp) integrationTest(NAME testKeyboardLayout SRCS keyboard_layout_test.cpp) +integrationTest(NAME testKeymapCreationFailure SRCS keymap_creation_failure_test.cpp) if (XCB_ICCCM_FOUND) integrationTest(NAME testMoveResize SRCS move_resize_window_test.cpp LIBS XCB::ICCCM) diff --git a/autotests/integration/keymap_creation_failure_test.cpp b/autotests/integration/keymap_creation_failure_test.cpp new file mode 100644 index 0000000000..dd8d7b6411 --- /dev/null +++ b/autotests/integration/keymap_creation_failure_test.cpp @@ -0,0 +1,102 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2017 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 "keyboard_input.h" +#include "keyboard_layout.h" +#include "platform.h" +#include "shell_client.h" +#include "virtualdesktops.h" +#include "wayland_server.h" +#include "workspace.h" + +#include +#include + +#include + +using namespace KWin; +using namespace KWayland::Client; + +static const QString s_socketName = QStringLiteral("wayland_test_kwin_keymap_creation_failure-0"); + +class KeymapCreationFailureTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void initTestCase(); + void init(); + void cleanup(); + + void testPointerButton(); +}; + +void KeymapCreationFailureTest::initTestCase() +{ + // situation for for BUG 381210 + // this will fail to create keymap + qputenv("XKB_DEFAULT_RULES", "no"); + qputenv("XKB_DEFAULT_MODEL", "no"); + qputenv("XKB_DEFAULT_LAYOUT", "no"); + qputenv("XKB_DEFAULT_VARIANT", "no"); + qputenv("XKB_DEFAULT_OPTIONS", "no"); + + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024)); + QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit())); + + kwinApp()->setConfig(KSharedConfig::openConfig(QString(), KConfig::SimpleConfig)); + kwinApp()->setKxkbConfig(KSharedConfig::openConfig(QString(), KConfig::SimpleConfig)); + KConfigGroup layoutGroup = kwinApp()->kxkbConfig()->group("Layout"); + layoutGroup.writeEntry("LayoutList", QStringLiteral("no")); + layoutGroup.writeEntry("Model", "no"); + layoutGroup.writeEntry("Options", "no"); + layoutGroup.sync(); + + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); + waylandServer()->initWorkspace(); +} + +void KeymapCreationFailureTest::init() +{ + QVERIFY(Test::setupWaylandConnection()); +} + +void KeymapCreationFailureTest::cleanup() +{ + Test::destroyWaylandConnection(); +} + +void KeymapCreationFailureTest::testPointerButton() +{ + // test case for BUG 381210 + // pressing a pointer button results in crash + + // now create the crashing condition + // which is sending in a pointer event + kwinApp()->platform()->pointerButtonPressed(BTN_LEFT, 0); + kwinApp()->platform()->pointerButtonReleased(BTN_LEFT, 1); +} + +WAYLANDTEST_MAIN(KeymapCreationFailureTest) +#include "keymap_creation_failure_test.moc" diff --git a/xkb.cpp b/xkb.cpp index f8dc6ed219..cdee22b68d 100644 --- a/xkb.cpp +++ b/xkb.cpp @@ -354,13 +354,16 @@ QString Xkb::layoutName() const QString Xkb::layoutName(xkb_layout_index_t layout) const { + if (!m_keymap) { + return QString{}; + } return QString::fromLocal8Bit(xkb_keymap_layout_get_name(m_keymap, layout)); } QMap Xkb::layoutNames() const { QMap layouts; - const auto size = xkb_keymap_num_layouts(m_keymap); + const auto size = m_keymap ? xkb_keymap_num_layouts(m_keymap) : 0u; for (xkb_layout_index_t i = 0; i < size; i++) { layouts.insert(i, layoutName(i)); } @@ -387,6 +390,9 @@ void Xkb::updateConsumedModifiers(uint32_t key) Qt::KeyboardModifiers Xkb::modifiersRelevantForGlobalShortcuts() const { + if (!m_state) { + return Qt::NoModifier; + } Qt::KeyboardModifiers mods = Qt::NoModifier; if (xkb_state_mod_index_is_active(m_state, m_shiftModifier, XKB_STATE_MODS_EFFECTIVE) == 1) { mods |= Qt::ShiftModifier;