Bug 481239 - KWin crash in KWaylandServer::TextInputV2Interface::setModifiersMap() during zwp_input_method_context_v1_modifiers_map
Summary: KWin crash in KWaylandServer::TextInputV2Interface::setModifiersMap() during ...
Status: RESOLVED FIXED
Alias: None
Product: kwin
Classification: Plasma
Component: wayland-generic (show other bugs)
Version: 5.27.10
Platform: Gentoo Packages Linux
: NOR crash
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-11 19:24 UTC by Bobby Bingham
Modified: 2024-02-15 14:28 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 5.27.11


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bobby Bingham 2024-02-11 19:24:54 UTC
SUMMARY

I've been periodically seeing a crash, usually when changing virtual desktops back to a desktop that has an instance of konsole running.  It doesn't always happen, and I haven't been able to find a way to reliably reproduce it, but I did preemptively attach gdb to kwin_wayland and captured some information the last time I hit the crash:

```
Thread 1 "kwin_wayland" received signal SIGSEGV, Segmentation fault.
0x00007fe2bf7ab72e in memcmp (vl=<optimized out>, vr=<optimized out>, n=24) at src/string/memcmp.c:6
6               for (; n && *l == *r; n--, l++, r++);
(gdb) bt
#0  0x00007fe2bf7ab72e in memcmp (vl=<optimized out>, vr=<optimized out>, n=24) at src/string/memcmp.c:6
#1  0x00007fe2bf0fef0d in operator== (a1=..., a2=...) at /usr/include/qt5/QtCore/qbytearray.h:685
#2  KWaylandServer::TextInputV2Interface::setModifiersMap (this=0x7fe2b80861a0, modifiersMap=...)
    at /usr/src/debug/kde-plasma/kwin-5.27.10-r2/kwin-5.27.10/src/wayland/textinput_v2_interface.cpp:524
#3  0x00007fe2bd6c46f4 in QtPrivate::QSlotObjectBase::call (a=0x7fff177da8a0, r=0x7fe2ae3a8490, this=0x7fe2a24fa9a0)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/include/QtCore/../../src/corelib/kernel/qobjectdefs_impl.h:398
#4  doActivate<false> (sender=0x7fe2abfbbfa0, signal_index=15, argv=0x7fff177da8a0)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qobject.cpp:3925
#5  0x00007fe2bd6be4c7 in QMetaObject::activate (sender=<optimized out>, m=m@entry=0x7fe2bf2bc300 <KWaylandServer::InputMethodContextV1Interface::staticMetaObject>,
    local_signal_index=local_signal_index@entry=12, argv=argv@entry=0x7fff177da8a0)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qobject.cpp:3985
#6  0x00007fe2bee13bc5 in KWaylandServer::InputMethodContextV1Interface::modifiersMap (this=<optimized out>, _t1=...)
    at /usr/src/debug/kde-plasma/kwin-5.27.10-r2/kwin-5.27.10_build/src/kwin_autogen/IEXH3JLKNG/moc_inputmethod_v1_interface.cpp:457
#7  0x00007fe2bf0bec86 in KWaylandServer::InputMethodContextV1InterfacePrivate::zwp_input_method_context_v1_modifiers_map (this=0x7fe2a3991f90, map=<optimized out>)
    at /usr/src/debug/kde-plasma/kwin-5.27.10-r2/kwin-5.27.10/src/wayland/inputmethod_v1_interface.cpp:116
#8  0x00007fe2bacfb3ca in ffi_call_unix64 () at /usr/src/debug/dev-libs/libffi-3.4.4-r4/libffi-3.4.4/src/x86/unix64.S:104
#9  0x00007fe2bacfa7e4 in ffi_call_int (cif=cif@entry=0x7fff177daad0,
    fn=fn@entry=0x7fe2bedf5600 <QtWaylandServer::zwp_input_method_context_v1::handle_modifiers_map(wl_client*, wl_resource*, wl_array*)>, rvalue=<optimized out>,
    rvalue@entry=0x0, avalue=<optimized out>, closure=closure@entry=0x0) at /usr/src/debug/dev-libs/libffi-3.4.4-r4/libffi-3.4.4/src/x86/ffi64.c:673
#10 0x00007fe2bacfaf3d in ffi_call (cif=cif@entry=0x7fff177daad0,
    fn=0x7fe2bedf5600 <QtWaylandServer::zwp_input_method_context_v1::handle_modifiers_map(wl_client*, wl_resource*, wl_array*)>, rvalue=rvalue@entry=0x0,
    avalue=avalue@entry=0x7fff177daba0) at /usr/src/debug/dev-libs/libffi-3.4.4-r4/libffi-3.4.4/src/x86/ffi64.c:710
#11 0x00007fe2bc1cbb14 in wl_closure_invoke (closure=0x7fe2a4ba7340, flags=<optimized out>, target=<optimized out>, opcode=7, data=<optimized out>)
    at ../wayland-1.22.0/src/connection.c:1025
#12 0x00007fe2bc1c6da3 in wl_client_connection_data (fd=<optimized out>, mask=<optimized out>, data=0x7fe2aa1d6200) at ../wayland-1.22.0/src/wayland-server.c:438
#13 0x00007fe2bc1c9bb2 in wl_event_loop_dispatch (loop=0x7fe2b80899d0, timeout=<optimized out>) at ../wayland-1.22.0/src/event-loop.c:1027
#14 0x00007fe2bf0b36b5 in KWaylandServer::Display::dispatchEvents (this=<optimized out>) at /usr/src/debug/kde-plasma/kwin-5.27.10-r2/kwin-5.27.10/src/wayland/display.cpp:114
#15 0x00007fe2bd6c46f4 in QtPrivate::QSlotObjectBase::call (a=0x7fff177db080, r=0x7fe2bbe913d0, this=0x7fe2ac844900)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/include/QtCore/../../src/corelib/kernel/qobjectdefs_impl.h:398
#16 doActivate<false> (sender=0x7fe2ac3e9920, signal_index=3, argv=0x7fff177db080)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qobject.cpp:3925
#17 0x00007fe2bd6be4c7 in QMetaObject::activate (sender=sender@entry=0x7fe2ac3e9920, m=m@entry=0x7fe2bd9671e0 <QSocketNotifier::staticMetaObject>,
    local_signal_index=local_signal_index@entry=0, argv=argv@entry=0x7fff177db080)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qobject.cpp:3985
#18 0x00007fe2bd6c7f7d in QSocketNotifier::activated (this=this@entry=0x7fe2ac3e9920, _t1=..., _t2=<optimized out>, _t3=...) at .moc/moc_qsocketnotifier.cpp:178
#19 0x00007fe2bd6c87ad in QSocketNotifier::event (this=0x7fe2ac3e9920, e=<optimized out>)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qsocketnotifier.cpp:302
#20 0x00007fe2bcb6323e in QApplicationPrivate::notify_helper (this=<optimized out>, receiver=0x7fe2ac3e9920, e=0x7fff177db190)
    at /usr/src/debug/dev-qt/qtwidgets-5.15.12/qtbase-everywhere-src-5.15.12/src/widgets/kernel/qapplication.cpp:3640
#21 0x00007fe2bd691628 in QCoreApplication::notifyInternal2 (receiver=0x7fe2ac3e9920, event=0x7fff177db190)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qcoreapplication.cpp:1064
#22 0x00007fe2bd69181e in QCoreApplication::sendEvent (receiver=<optimized out>, event=<optimized out>)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qcoreapplication.cpp:1462
#23 0x00007fe2bd6e039e in QEventDispatcherUNIXPrivate::activateSocketNotifiers (this=this@entry=0x7fe2be49aeb0)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qeventdispatcher_unix.cpp:304
#24 0x00007fe2bd6e079c in QEventDispatcherUNIX::processEvents (this=<optimized out>, flags=...)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/src/corelib/kernel/qeventdispatcher_unix.cpp:511
#25 0x0000561ea79554dd in QUnixEventDispatcherQPA::processEvents (this=<optimized out>, flags=...)
    at /var/tmp/portage/dev-qt/qtgui-5.15.12/work/qtbase-everywhere-src-5.15.12/src/platformsupport/eventdispatchers/qunixeventdispatcher.cpp:63
#26 0x00007fe2bd69004b in QEventLoop::exec (this=this@entry=0x7fff177db300, flags=..., flags@entry=...)
    at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/include/QtCore/../../src/corelib/global/qflags.h:69
#27 0x00007fe2bd6983cd in QCoreApplication::exec () at /usr/src/debug/dev-qt/qtcore-5.15.12-r1/qtbase-everywhere-src-5.15.12/include/QtCore/../../src/corelib/global/qflags.h:121
#28 0x00007fe2bdb1ba4c in QGuiApplication::exec () at /usr/src/debug/dev-qt/qtgui-5.15.12/qtbase-everywhere-src-5.15.12/src/gui/kernel/qguiapplication.cpp:1870
#29 0x00007fe2bcb631b5 in QApplication::exec () at /usr/src/debug/dev-qt/qtwidgets-5.15.12/qtbase-everywhere-src-5.15.12/src/widgets/kernel/qapplication.cpp:2832
#30 0x0000561ea786e012 in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/kde-plasma/kwin-5.27.10-r2/kwin-5.27.10/src/main_wayland.cpp:628
```

It looks like in `QByteArray::operator==`, the rhs of the comparison is fine, but the lhs has a dangling pointer to some inaccessible memory:

```
(gdb) up
#1  0x00007fe2bf0fef0d in operator== (a1=..., a2=...) at /usr/include/qt5/QtCore/qbytearray.h:685
685     { return (a1.size() == a2.size()) && (memcmp(a1.constData(), a2.constData(), a1.size())==0); }
(gdb) p (((char*)a2->d) + a2->d->offset)[0]@a2->d->size
$14 = "Shift\000Control\000Mod1\000Mod4"
(gdb) p (((char*)a1->d) + a1->d->offset)[0]@a1->d->size
Cannot access memory at address 0x7fe2a23859c4
```

This is called by TextInputV2Interface::setModifiersMap[1]:

```
void TextInputV2Interface::setModifiersMap(const QByteArray &modifiersMap)
{
    if (d->modifiersMap == modifiersMap) {
        // not changed
        return;
    }
    d->modifiersMap = modifiersMap;
    d->sendModifiersMap();
}
```

So, somehow the existing data in `d->modifiersMap` is bad.

Looking further up the call stack, I see KWaylandServer::InputMethodContextV1InterfacePrivate::zwp_input_method_context_v1_modifiers_map[2]:

```
    void zwp_input_method_context_v1_modifiers_map(Resource *, wl_array *map) override
    {
        const auto mods = QByteArray::fromRawData(static_cast<const char *>(map->data), map->size);

        Q_EMIT q->modifiersMap(mods);
    }
```

I'm not familiar with the Qt or the Wayland APIs involved here.  But as best I can tell, `QByteArray::fromRawData` doesn't copy the provided data, but stores a pointer to the provided data, meaning the data needs to have a lifetime at least as long as the `QByteArray` being created.

I don't know if that's guaranteed for the `map` parameter passed to this function.  Maybe this gets called multiple times, and when it's called the second time, the pointer provided in the first call (which was saved off) is no longer valid?

[1]: https://invent.kde.org/plasma/kwin/-/blob/c0e1f817668b6ab62030168eafa0ef266250f006/src/wayland/textinput_v2_interface.cpp#L524
[2]: https://invent.kde.org/plasma/kwin/-/blob/c0e1f817668b6ab62030168eafa0ef266250f006/src/wayland/inputmethod_v1_interface.cpp#L116


STEPS TO REPRODUCE
Not entirely sure.  Happens sometimes when switching virtual desktop to one where an instance of konsole is running.

SOFTWARE VERSIONS
KDE Plasma Version: 5.27.10
KDE Frameworks Version: 5.114.0
Qt Version: 5.15.12

ADDITIONAL INFORMATION
- My system is running musl, rather than glibc, though that's probably not the issue here.
- I'm using fcitx5 as an input method
Comment 1 Zamundaaa 2024-02-14 21:29:25 UTC
Good investigation. Afaik the wl_array data is deleted again after the callback... https://invent.kde.org/plasma/kwin/-/merge_requests/5203 should address that.
Comment 2 Vlad Zahorodnii 2024-02-15 12:21:59 UTC
Git commit 3a8ae60f87968ba2d16454b331e807b5127bcc85 by Vlad Zahorodnii, on behalf of Xaver Hugl.
Committed on 15/02/2024 at 12:07.
Pushed by vladz into branch 'master'.

wayland/textinput_v2: copy the data instead of assuming ownership

The life time of the wl_array is limited to the callback, afterwards it's
a dangling pointer

M  +1    -1    src/wayland/inputmethod_v1.cpp

https://invent.kde.org/plasma/kwin/-/commit/3a8ae60f87968ba2d16454b331e807b5127bcc85
Comment 3 Vlad Zahorodnii 2024-02-15 12:32:37 UTC
Git commit 60849214b3968b779f60f53c8592e1fd6ce2d8af by Vlad Zahorodnii, on behalf of Xaver Hugl.
Committed on 15/02/2024 at 12:23.
Pushed by vladz into branch 'Plasma/6.0'.

wayland/textinput_v2: copy the data instead of assuming ownership

The life time of the wl_array is limited to the callback, afterwards it's
a dangling pointer


(cherry picked from commit 3a8ae60f87968ba2d16454b331e807b5127bcc85)

M  +1    -1    src/wayland/inputmethod_v1.cpp

https://invent.kde.org/plasma/kwin/-/commit/60849214b3968b779f60f53c8592e1fd6ce2d8af
Comment 4 Vlad Zahorodnii 2024-02-15 12:37:44 UTC
Git commit 2f0faf45858d6dbafa00875e584bc1f6b1861b3a by Vlad Zahorodnii, on behalf of Xaver Hugl.
Committed on 15/02/2024 at 12:23.
Pushed by vladz into branch 'Plasma/5.27'.

wayland/textinput_v2: copy the data instead of assuming ownership

The life time of the wl_array is limited to the callback, afterwards it's
a dangling pointer


(cherry picked from commit 3a8ae60f87968ba2d16454b331e807b5127bcc85)

M  +1    -1    src/wayland/inputmethod_v1_interface.cpp

https://invent.kde.org/plasma/kwin/-/commit/2f0faf45858d6dbafa00875e584bc1f6b1861b3a