From b740083d9a2edd725a69e274e8d32874e7425405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= Date: Fri, 30 Jun 2017 20:10:49 +0200 Subject: [PATCH] [server] Pass a dedicated fd to each keyboard for the xkb keymap To better isolate the clients from each other eachh KeyboardInterface creates it's own dedicated temporary file and sends the fd for this temporary file to the client. This means the memory for the keymap is no longer shared between all clients, every client has an own copy. To support this the existing API to set the keymap is deprecated and replaced by a new method setKeymapData which takes the content of the keymap as a byte array. The now deprecated method which takes a file descriptor is changed to use the new setKeymapData method. For that it reads the content of the file. The implementation in KeyboardInterface to create the file descriptor is based on the implementation of KWin. As I implemented the change in KWin (see 3b4c508ee36ac74c37e77fcaa14d106397ad2994) it is not a problem from GPL vs LGPL perspective. The change includes test cases to verify that the content of the keymap is properly passed to the client and that the memory is no longer shared. BUG: 381674 --- autotests/client/test_wayland_seat.cpp | 96 +++++++++++++++++++++++++++++++++- src/server/keyboard_interface.cpp | 26 +++++++++ src/server/keyboard_interface.h | 1 + src/server/keyboard_interface_p.h | 3 ++ src/server/seat_interface.cpp | 32 +++++++++--- src/server/seat_interface.h | 27 ++++++++-- src/server/seat_interface_p.h | 3 +- 7 files changed, 173 insertions(+), 15 deletions(-) diff --git a/autotests/client/test_wayland_seat.cpp b/autotests/client/test_wayland_seat.cpp index 26e2152..0099ce9 100644 --- a/autotests/client/test_wayland_seat.cpp +++ b/autotests/client/test_wayland_seat.cpp @@ -90,7 +90,8 @@ private Q_SLOTS: void testTouch(); void testDisconnect(); void testPointerEnterOnUnboundSurface(); - // TODO: add test for keymap + void testKeymap(); + void testKeymapThroughFd(); private: KWayland::Server::Display *m_display; @@ -1595,7 +1596,6 @@ void TestWaylandSeat::testKeyboard() m_seatInterface->setKeyRepeatInfo(30, 560); m_seatInterface->setKeyRepeatInfo(25, 660); m_seatInterface->updateKeyboardModifiers(5, 6, 7, 8); - m_seatInterface->setKeymap(open("/dev/null", O_RDONLY), 0); m_seatInterface->setFocusedKeyboardSurface(nullptr); m_seatInterface->setFocusedKeyboardSurface(serverSurface); QCOMPARE(m_seatInterface->focusedKeyboardSurface(), serverSurface); @@ -2285,5 +2285,97 @@ void TestWaylandSeat::testPointerEnterOnUnboundSurface() QVERIFY(!clientErrorSpy.wait(100)); } +void TestWaylandSeat::testKeymap() +{ + using namespace KWayland::Client; + using namespace KWayland::Server; + + m_seatInterface->setHasKeyboard(true); + QSignalSpy keyboardChangedSpy(m_seat, &Seat::hasKeyboardChanged); + QVERIFY(keyboardChangedSpy.isValid()); + QVERIFY(keyboardChangedSpy.wait()); + + QScopedPointer keyboard(m_seat->createKeyboard()); + QSignalSpy keymapChangedSpy(keyboard.data(), &Keyboard::keymapChanged); + QVERIFY(keymapChangedSpy.isValid()); + + m_seatInterface->setKeymapData(QByteArrayLiteral("foo")); + QVERIFY(keymapChangedSpy.wait()); + int fd = keymapChangedSpy.first().first().toInt(); + QVERIFY(fd != -1); + QCOMPARE(keymapChangedSpy.first().last().value(), 3u); + QFile file; + QVERIFY(file.open(fd, QIODevice::ReadOnly)); + const char *address = reinterpret_cast(file.map(0, keymapChangedSpy.first().last().value())); + QVERIFY(address); + QCOMPARE(qstrcmp(address, "foo"), 0); + file.close(); + + // change the keymap + keymapChangedSpy.clear(); + m_seatInterface->setKeymapData(QByteArrayLiteral("bar")); + QVERIFY(keymapChangedSpy.wait()); + fd = keymapChangedSpy.first().first().toInt(); + QVERIFY(fd != -1); + QCOMPARE(keymapChangedSpy.first().last().value(), 3u); + QVERIFY(file.open(fd, QIODevice::ReadWrite));address = reinterpret_cast(file.map(0, keymapChangedSpy.first().last().value())); + QVERIFY(address); + QCOMPARE(qstrcmp(address, "bar"), 0); +} + +void TestWaylandSeat::testKeymapThroughFd() +{ +#ifndef KWAYLANDSERVER_NO_DEPRECATED + using namespace KWayland::Client; + using namespace KWayland::Server; + + m_seatInterface->setHasKeyboard(true); + QSignalSpy keyboardChangedSpy(m_seat, &Seat::hasKeyboardChanged); + QVERIFY(keyboardChangedSpy.isValid()); + QVERIFY(keyboardChangedSpy.wait()); + + QScopedPointer keyboard(m_seat->createKeyboard()); + QSignalSpy keymapChangedSpy(keyboard.data(), &Keyboard::keymapChanged); + QVERIFY(keymapChangedSpy.isValid()); + + QTemporaryFile serverFile; + QVERIFY(serverFile.open()); + QByteArray data = QByteArrayLiteral("foobar"); + QVERIFY(serverFile.resize(data.size() + 1)); + uchar *serverAddress = serverFile.map(0, data.size() + 1); + QVERIFY(serverAddress); + QVERIFY(qstrncpy(reinterpret_cast(serverAddress), data.constData(), data.size() + 1)); + m_seatInterface->setKeymap(serverFile.handle(), data.size()); + + + QVERIFY(keymapChangedSpy.wait()); + int fd = keymapChangedSpy.first().first().toInt(); + QVERIFY(fd != -1); + QCOMPARE(keymapChangedSpy.first().last().value(), 6u); + QFile file; + QVERIFY(file.open(fd, QIODevice::ReadOnly)); + const char *address = reinterpret_cast(file.map(0, keymapChangedSpy.first().last().value())); + QVERIFY(address); + QCOMPARE(qstrcmp(address, "foobar"), 0); + + QScopedPointer keyboard2(m_seat->createKeyboard()); + QSignalSpy keymapChangedSpy2(keyboard2.data(), &Keyboard::keymapChanged); + QVERIFY(keymapChangedSpy2.isValid()); + QVERIFY(keymapChangedSpy2.wait()); + + int fd2 = keymapChangedSpy2.first().first().toInt(); + QVERIFY(fd2 != -1); + QCOMPARE(keymapChangedSpy2.first().last().value(), 6u); + QFile file2; + QVERIFY(file2.open(fd2, QIODevice::ReadWrite)); + char *address2 = reinterpret_cast(file2.map(0, keymapChangedSpy2.first().last().value())); + QVERIFY(address2); + QCOMPARE(qstrcmp(address2, "foobar"), 0); + address2[0] = 'g'; + QCOMPARE(qstrcmp(address2, "goobar"), 0); + QCOMPARE(qstrcmp(address, "foobar"), 0); +#endif +} + QTEST_GUILESS_MAIN(TestWaylandSeat) #include "test_wayland_seat.moc" diff --git a/src/server/keyboard_interface.cpp b/src/server/keyboard_interface.cpp index a5ffd8a..464c533 100644 --- a/src/server/keyboard_interface.cpp +++ b/src/server/keyboard_interface.cpp @@ -23,10 +23,13 @@ License along with this library. If not, see . #include "seat_interface.h" #include "surface_interface.h" // Qt +#include #include // Wayland #include +#include + namespace KWayland { @@ -90,6 +93,29 @@ void KeyboardInterface::setKeymap(int fd, quint32 size) d->sendKeymap(fd, size); } +void KeyboardInterface::setKeymap(const QByteArray &content) +{ + QScopedPointer tmp{new QTemporaryFile(this)}; + if (!tmp->open()) { + return; + } + unlink(tmp->fileName().toUtf8().constData()); + if (!tmp->resize(content.size())) { + return; + } + uchar *address = tmp->map(0, content.size()); + if (!address) { + return; + } + if (qstrncpy(reinterpret_cast(address), content.constData(), content.size() + 1) == nullptr) { + return; + } + tmp->unmap(address); + Q_D(); + d->sendKeymap(tmp->handle(), content.size()); + d->keymap.swap(tmp); +} + void KeyboardInterface::Private::sendKeymap(int fd, quint32 size) { if (!resource) { diff --git a/src/server/keyboard_interface.h b/src/server/keyboard_interface.h index 17d1d20..099b2cd 100644 --- a/src/server/keyboard_interface.h +++ b/src/server/keyboard_interface.h @@ -50,6 +50,7 @@ public: private: void setFocusedSurface(SurfaceInterface *surface, quint32 serial); void setKeymap(int fd, quint32 size); + void setKeymap(const QByteArray &content); void updateModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, quint32 serial); void keyPressed(quint32 key, quint32 serial); void keyReleased(quint32 key, quint32 serial); diff --git a/src/server/keyboard_interface_p.h b/src/server/keyboard_interface_p.h index c7fba23..3bdeda1 100644 --- a/src/server/keyboard_interface_p.h +++ b/src/server/keyboard_interface_p.h @@ -24,6 +24,8 @@ License along with this library. If not, see . #include +class QTemporaryFile; + namespace KWayland { namespace Server @@ -45,6 +47,7 @@ public: SurfaceInterface *focusedSurface = nullptr; QPointer focusedChildSurface; QMetaObject::Connection destroyConnection; + QScopedPointer keymap; private: static const struct wl_keyboard_interface s_interface; diff --git a/src/server/seat_interface.cpp b/src/server/seat_interface.cpp index 3b17982..94dfa3a 100644 --- a/src/server/seat_interface.cpp +++ b/src/server/seat_interface.cpp @@ -491,7 +491,7 @@ void SeatInterface::Private::getKeyboard(wl_client *client, wl_resource *resourc } keyboard->repeatInfo(keys.keyRepeat.charactersPerSecond, keys.keyRepeat.delay); if (keys.keymap.xkbcommonCompatible) { - keyboard->setKeymap(keys.keymap.fd, keys.keymap.size); + keyboard->setKeymap(keys.keymap.content); } keyboards << keyboard; if (keys.focus.surface && keys.focus.surface->client() == clientConnection) { @@ -1087,14 +1087,28 @@ void SeatInterface::setFocusedKeyboardSurface(SurfaceInterface *surface) } } +#ifndef KWAYLANDSERVER_NO_DEPRECATED void SeatInterface::setKeymap(int fd, quint32 size) { + QFile file; + if (!file.open(fd, QIODevice::ReadOnly)) { + return; + } + const char *address = reinterpret_cast(file.map(0, size)); + if (!address) { + return; + } + setKeymapData(QByteArray(address, size)); +} +#endif + +void SeatInterface::setKeymapData(const QByteArray &content) +{ Q_D(); d->keys.keymap.xkbcommonCompatible = true; - d->keys.keymap.fd = fd; - d->keys.keymap.size = size; + d->keys.keymap.content = content; for (auto it = d->keyboards.constBegin(); it != d->keyboards.constEnd(); ++it) { - (*it)->setKeymap(fd, size); + (*it)->setKeymap(content); } } @@ -1151,17 +1165,19 @@ bool SeatInterface::isKeymapXkbCompatible() const return d->keys.keymap.xkbcommonCompatible; } +#ifndef KWAYLANDSERVER_NO_DEPRECATED int SeatInterface::keymapFileDescriptor() const { - Q_D(); - return d->keys.keymap.fd; + return -1; } +#endif +#ifndef KWAYLANDSERVER_NO_DEPRECATED quint32 SeatInterface::keymapSize() const { - Q_D(); - return d->keys.keymap.size; + return 0; } +#endif quint32 SeatInterface::depressedModifiers() const { diff --git a/src/server/seat_interface.h b/src/server/seat_interface.h index bf11680..9dd4f16 100644 --- a/src/server/seat_interface.h +++ b/src/server/seat_interface.h @@ -518,7 +518,18 @@ public: * @name keyboard related methods **/ ///@{ - void setKeymap(int fd, quint32 size); + /** + * @deprecated Since 5.KEYMAPFIX use setKeymapData + **/ +#ifndef KWAYLANDSERVER_NO_DEPRECATED + void KWAYLANDSERVER_DEPRECATED setKeymap(int fd, quint32 size); +#endif + /** + * Sets the xkb keymap with @p content for this Seat. + * The content gets sent to all registered KeyboardInterfaces + * @since 5.KEYMAPFIX + **/ + void setKeymapData(const QByteArray &content); void keyPressed(quint32 key); void keyReleased(quint32 key); void updateKeyboardModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group); @@ -540,8 +551,18 @@ public: quint32 lockedModifiers() const; quint32 groupModifiers() const; quint32 lastModifiersSerial() const; - int keymapFileDescriptor() const; - quint32 keymapSize() const; + /** + * @deprecated since 5.KEYMAPFIX + **/ +#ifndef KWAYLANDSERVER_NO_DEPRECATED + int KWAYLANDSERVER_DEPRECATED keymapFileDescriptor() const; +#endif + /** + * @deprecated since 5.KEYMAPFIX + **/ +#ifndef KWAYLANDSERVER_NO_DEPRECATED + quint32 KWAYLANDSERVER_DEPRECATED keymapSize() const; +#endif bool isKeymapXkbCompatible() const; QVector pressedKeys() const; /** diff --git a/src/server/seat_interface_p.h b/src/server/seat_interface_p.h index f4c4496..d26fffc 100644 --- a/src/server/seat_interface_p.h +++ b/src/server/seat_interface_p.h @@ -99,9 +99,8 @@ public: }; QHash states; struct Keymap { - int fd = -1; - quint32 size = 0; bool xkbcommonCompatible = false; + QByteArray content; }; Keymap keymap; struct Modifiers { -- 2.7.4