Commit a433fb08 authored by Vlad Zahorodnii's avatar Vlad Zahorodnii

x11: Make removal of X11 event filters safe

If an X11 event filter has been activated and it unregisters another X11
event filter, then the window manager may crash because the foreach macro
in Workspace::workspaceEvent() makes a copy of m_genericEventFilters or
m_eventFilters and we can call the event() method for an already defunct
filter.

With this change, X11 event filters can be safely removed and installed
at any particular moment.

BUG: 423319
parent 260e75f6
......@@ -165,18 +165,34 @@ QVector<QByteArray> s_xcbEerrors({
void Workspace::registerEventFilter(X11EventFilter *filter)
{
if (filter->isGenericEvent())
m_genericEventFilters.append(filter);
else
m_eventFilters.append(filter);
if (filter->isGenericEvent()) {
m_genericEventFilters.append(new X11EventFilterContainer(filter));
} else {
m_eventFilters.append(new X11EventFilterContainer(filter));
}
}
static X11EventFilterContainer *takeEventFilter(X11EventFilter *eventFilter,
QList<QPointer<X11EventFilterContainer>> &list)
{
for (int i = 0; i < list.count(); ++i) {
X11EventFilterContainer *container = list.at(i);
if (container->filter() == eventFilter) {
return list.takeAt(i);
}
}
return nullptr;
}
void Workspace::unregisterEventFilter(X11EventFilter *filter)
{
if (filter->isGenericEvent())
m_genericEventFilters.removeOne(filter);
else
m_eventFilters.removeOne(filter);
X11EventFilterContainer *container = nullptr;
if (filter->isGenericEvent()) {
container = takeEventFilter(filter, m_genericEventFilters);
} else {
container = takeEventFilter(filter, m_eventFilters);
}
delete container;
}
......@@ -219,13 +235,29 @@ bool Workspace::workspaceEvent(xcb_generic_event_t *e)
if (eventType == XCB_GE_GENERIC) {
xcb_ge_generic_event_t *ge = reinterpret_cast<xcb_ge_generic_event_t *>(e);
foreach (X11EventFilter *filter, m_genericEventFilters) {
// We need to make a shadow copy of the event filter list because an activated event
// filter may mutate it by removing or installing another event filter.
const auto eventFilters = m_genericEventFilters;
for (X11EventFilterContainer *container : eventFilters) {
if (!container) {
continue;
}
X11EventFilter *filter = container->filter();
if (filter->extension() == ge->extension && filter->genericEventTypes().contains(ge->event_type) && filter->event(e)) {
return true;
}
}
} else {
foreach (X11EventFilter *filter, m_eventFilters) {
// We need to make a shadow copy of the event filter list because an activated event
// filter may mutate it by removing or installing another event filter.
const auto eventFilters = m_eventFilters;
for (X11EventFilterContainer *container : eventFilters) {
if (!container) {
continue;
}
X11EventFilter *filter = container->filter();
if (filter->eventTypes().contains(eventType) && filter->event(e)) {
return true;
}
......
......@@ -66,6 +66,16 @@ namespace KWin
extern int screen_number;
extern bool is_multihead;
X11EventFilterContainer::X11EventFilterContainer(X11EventFilter *filter)
: m_filter(filter)
{
}
X11EventFilter *X11EventFilterContainer::filter() const
{
return m_filter;
}
ColorMapper::ColorMapper(QObject *parent)
: QObject(parent)
, m_default(kwinApp()->x11DefaultScreen()->default_colormap)
......
......@@ -55,6 +55,19 @@ class X11Client;
class X11EventFilter;
enum class Predicate;
class X11EventFilterContainer : public QObject
{
Q_OBJECT
public:
explicit X11EventFilterContainer(X11EventFilter *filter);
X11EventFilter *filter() const;
private:
X11EventFilter *m_filter;
};
class KWIN_EXPORT Workspace : public QObject
{
Q_OBJECT
......@@ -654,8 +667,8 @@ private:
QScopedPointer<KillWindow> m_windowKiller;
QList<X11EventFilter *> m_eventFilters;
QList<X11EventFilter *> m_genericEventFilters;
QList<QPointer<X11EventFilterContainer>> m_eventFilters;
QList<QPointer<X11EventFilterContainer>> m_genericEventFilters;
QScopedPointer<X11EventFilter> m_movingClientFilter;
QScopedPointer<X11EventFilter> m_syncAlarmFilter;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment