https://bugs.kde.org/show_bug.cgi?id=523770
Bug ID: 523770
Summary: Use-after-free in DpmsInputEventFilter::touchUp: the
filter writes to its own members after notify() has
destroyed it
Classification: Plasma
Product: kwin
Version First 6.6.6
Reported In:
Platform: Kubuntu
OS: Linux
Status: REPORTED
Severity: crash
Priority: NOR
Component: input
Assignee: [email protected]
Reporter: [email protected]
CC: [email protected]
Target Milestone: ---
Created attachment 194798
--> https://bugs.kde.org/attachment.cgi?id=194798&action=edit
Full backtrace and heap state from one of the core dumps
Still present in current master. The crashes reported here were observed on
6.6.5 and
6.6.6, which is what the core dumps below come from. The offending code is
byte-identical
in current master (6.7.80) — the sources quoted here are taken from master, not
from the
version the dumps came from.
SUMMARY
DpmsInputEventFilter::touchUp() calls notify() and then assigns to two of its
own
members. notify() destroys the filter synchronously, so both assignments write
into freed memory. On a touch-only system this happens on every double-tap
wake-up and eventually corrupts the heap.
bool DpmsInputEventFilter::touchUp(TouchUpEvent *event)
{
if (m_enableDoubleTap) {
m_touchPoints.removeAll(event->id);
if (m_touchPoints.isEmpty() && m_doubleTapTimer.isValid() &&
m_secondTap) {
// if device in pocket, do not wake device up
if (m_doubleTapTimer.elapsed() < qApp->doubleClickInterval() &&
!m_proximityClose) {
notify(); // destroys *this*
}
m_doubleTapTimer.invalidate(); // write to freed memory
m_secondTap = false; // write to freed memory
}
}
return true;
}
The destruction is entirely synchronous, within one call stack:
DpmsInputEventFilter::notify()
-> Workspace::requestDpmsState(DpmsState::On)
-> m_dpmsFilter.reset()
-> ~DpmsInputEventFilter()
-> ~InputEventFilter()
-> InputRedirection::uninstallInputEventFilter(this)
-> operator delete
The other handlers are fine: pointerMotion, pointerButton, pointerAxis and
keyboardKey call notify() and return immediately without touching any member.
touchUp is the only one that keeps using the object.
STEPS TO REPRODUCE
1. Wayland session on a system whose only input device is a touchscreen, with
DoubleTapWakeup at its default of true.
2. Let power management switch the outputs off via DPMS.
3. Wake the screen with a double tap.
Every such wake-up performs two writes into freed memory.
Why this mostly affects touch-only systems: while the outputs are off, a single
tap does not wake them, because touchDown/touchUp only accumulate touch points
and return true. The double tap is the only way to wake the screen through the
touchscreen, so on a device without keyboard or mouse the only available
wake-up gesture is exactly the broken code path. On a desktop the user nudges
the mouse instead, and pointerMotion is safe.
OBSERVED RESULT
The stray writes usually land in unused freed memory and do nothing. Only when
that region has been handed out again do they overwrite live heap metadata.
glibc then aborts at the next allocation that touches the damaged chunk -
minutes to days later, in an entirely unrelated code path. With the default
configuration this took roughly 6 to 9 days per machine.
Two aborts from the same installation, showing how far the reported location is
from the actual cause:
malloc_printerr ("corrupted size vs. prev_size")
unlink_chunk / _int_malloc / __libc_malloc2 / operator new
KWin::Transaction::watchDmaBuf
KWin::Transaction::commit
wl_event_loop_dispatch
KWin::Display::dispatchEvents
malloc_printerr ("corrupted size vs. prev_size")
unlink_chunk / _int_malloc / realloc / QArrayData::reallocateUnaligned
KWin::ItemRendererOpenGL::createRenderNode
KWin::ItemRendererOpenGL::renderItem
KWin::WorkspaceScene::paint
KWin::Compositor::composite
Anyone hitting this will report it as a random compositing or rendering crash.
Searching Bugzilla for "corrupted size vs. prev_size" in kwin returns nothing,
which fits the real cause never having been identified.
EXPECTED RESULT
No write to the object after notify() has destroyed it, and no crash.
SOFTWARE/OS VERSIONS
KDE Plasma: 6.6.5 and 6.6.6 (both affected, core dumps from both)
KWin: still affected in current master (6.7.80) - the code quoted above is
taken
from master, not from the versions that produced the dumps
Qt: 6.10.2
Operating System: Linux, Wayland session
ADDITIONAL INFORMATION
Evidence from core dumps
------------------------
Three core dumps from two different machines (different CPU generation,
chipset,
GPU and display connector), kwin 6.6.5 and 6.6.6. In every one of them:
1. Exactly one chunk in the whole heap has a corrupted prev_size. The heap is
otherwise fully walkable, around 150000 chunks from start to end.
2. That chunk is free and still contains a destroyed InputEventFilter.
3. The object's vptr is the vtable of InputEventFilter itself, not of a derived
class - the destructor chain had reached the base when the memory was
released.
4. m_weight == 1 == InputFilterOrder::Dpms identifies it as the DPMS filter.
5. Its QElapsedTimer members hold 0x8000000000000000 twice, which is precisely
what QElapsedTimer::invalidate() writes. That is the post-free write itself,
preserved in the dump.
Example from the 6.6.6 dump, object at offset 0x90 of a free 0xd0-byte chunk:
+0x00 0x00007f........ vtable for KWin::InputEventFilter
+0x08 0x0032000000000001 m_weight = 1 (InputFilterOrder::Dpms)
+0x10 0x8000000000000000 QElapsedTimer t1 } written by invalidate()
+0x18 0x8000000000000000 QElapsedTimer t2 } after the free
This can also be observed without waiting for a crash: right after a double-tap
wake-up, walk kwin's heap and look for a free chunk that still contains an
InputEventFilter whose m_weight is 1 and whose two timer fields both read
0x8000000000000000. An ASan build should flag the write immediately.
Secondary issue in the same area
--------------------------------
InputRedirection::processFilters() iterates over m_filters itself while a
filter
is allowed to destroy itself - and ~InputEventFilter removes it from that very
list:
void processFilters(auto method, const auto &...args)
{
for (const auto filter : std::as_const(m_filters)) {
if ((filter->*method)(args...)) {
return;
}
}
}
This stays benign only because every handler that calls notify() returns true,
which ends the loop at once - with one exception. DpmsInputEventFilter::
switchEvent() calls notify() on a lid switch and then returns false, so the
loop
continues over a list that was just modified underneath it.
Proposed fix
------------
Two changes, kept separate so the first can be taken on its
own:
1. dpmsinputeventfilter: reset the filter's state before calling notify(), so
that notify() is the last statement touching the object.
2. input: make processFilters tolerate filters destroyed during dispatch, by
iterating over a copy and skipping filters that are no longer installed.
--
You are receiving this mail because:
You are watching all bug changes.
