1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
|
From c5880833d94330d022c4b6fc84c175aadeaf9632 Mon Sep 17 00:00:00 2001
From: Vlad Zahorodnii <vlad.zahorodnii@kde.org>
Date: Tue, 22 Sep 2020 08:53:17 +0000
Subject: [PATCH] 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
(cherry picked from commit a433fb08a3a9255802405a17dd4c8270c68fcb25)
---
events.cpp | 52 +++++++++++++++++++++++++++++++++++++++++----------
workspace.cpp | 10 ++++++++++
workspace.h | 17 +++++++++++++++--
3 files changed, 67 insertions(+), 12 deletions(-)
diff --git a/events.cpp b/events.cpp
index eb3572d13..2e8885d76 100644
--- a/events.cpp
+++ b/events.cpp
@@ -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;
}
diff --git a/workspace.cpp b/workspace.cpp
index a87a622e9..fd3634b16 100644
--- a/workspace.cpp
+++ b/workspace.cpp
@@ -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)
diff --git a/workspace.h b/workspace.h
index 489d7bae4..61fb215a8 100644
--- a/workspace.h
+++ b/workspace.h
@@ -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;
--
GitLab
|
