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
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
|
From d553d92d6e9f53cbe5a34166fcb919ba652c6a8e Mon Sep 17 00:00:00 2001
From: Patrick Griffis <pgriffis@igalia.com>
Date: Tue, 29 Jan 2019 10:07:06 -0500
Subject: [PATCH] gsocketclient: Fix criticals
This ensures the parent GTask is kept alive as long as an enumeration
is running and trying to connect.
Closes #1646
Closes #1649
Upstream-Status: Backport
CVE: CVE-2019-9633 patch 2
Affects: < 2.59.2
Signed-off-by: Armin Kuster <akuster@mvista.com>
---
gio/gsocketclient.c | 74 +++++++++++++++++++++++++++++-------------
gio/tests/gsocketclient-slow.c | 55 ++++++++++++++++++++++++++++++-
2 files changed, 106 insertions(+), 23 deletions(-)
Index: glib-2.58.0/gio/gsocketclient.c
===================================================================
--- glib-2.58.0.orig/gio/gsocketclient.c
+++ glib-2.58.0/gio/gsocketclient.c
@@ -1327,7 +1327,7 @@ g_socket_client_connect_to_uri (GSocketC
typedef struct
{
- GTask *task;
+ GTask *task; /* unowned */
GSocketClient *client;
GSocketConnectable *connectable;
@@ -1345,6 +1345,7 @@ static void connection_attempt_unref (gp
static void
g_socket_client_async_connect_data_free (GSocketClientAsyncConnectData *data)
{
+ data->task = NULL;
g_clear_object (&data->connectable);
g_clear_object (&data->enumerator);
g_clear_object (&data->proxy_addr);
@@ -1444,13 +1445,19 @@ set_last_error (GSocketClientAsyncConnec
}
static void
-enumerator_next_async (GSocketClientAsyncConnectData *data)
+enumerator_next_async (GSocketClientAsyncConnectData *data,
+ gboolean add_task_ref)
{
/* We need to cleanup the state */
g_clear_object (&data->socket);
g_clear_object (&data->proxy_addr);
g_clear_object (&data->connection);
+ /* Each enumeration takes a ref. This arg just avoids repeated unrefs when
+ an enumeration starts another enumeration */
+ if (add_task_ref)
+ g_object_ref (data->task);
+
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_RESOLVING, data->connectable, NULL);
g_socket_address_enumerator_next_async (data->enumerator,
g_task_get_cancellable (data->task),
@@ -1478,7 +1485,7 @@ g_socket_client_tls_handshake_callback (
else
{
g_object_unref (object);
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
}
}
@@ -1509,7 +1516,7 @@ g_socket_client_tls_handshake (GSocketCl
}
else
{
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
}
}
@@ -1530,13 +1537,24 @@ g_socket_client_proxy_connect_callback (
}
else
{
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
return;
}
g_socket_client_tls_handshake (data);
}
+static gboolean
+task_completed_or_cancelled (GTask *task)
+{
+ if (g_task_get_completed (task))
+ return TRUE;
+ else if (g_task_return_error_if_cancelled (task))
+ return TRUE;
+ else
+ return FALSE;
+}
+
static void
g_socket_client_connected_callback (GObject *source,
GAsyncResult *result,
@@ -1549,8 +1567,7 @@ g_socket_client_connected_callback (GObj
GProxy *proxy;
const gchar *protocol;
- /* data is NULL once the task is completed */
- if (data && g_task_return_error_if_cancelled (data->task))
+ if (g_cancellable_is_cancelled (attempt->cancellable) || task_completed_or_cancelled (data->task))
{
g_object_unref (data->task);
connection_attempt_unref (attempt);
@@ -1570,17 +1587,15 @@ g_socket_client_connected_callback (GObj
{
clarify_connect_error (error, data->connectable, attempt->address);
set_last_error (data, error);
+ connection_attempt_remove (attempt);
+ enumerator_next_async (data, FALSE);
}
else
- g_clear_error (&error);
-
- if (data)
{
- connection_attempt_remove (attempt);
- enumerator_next_async (data);
+ g_clear_error (&error);
+ g_object_unref (data->task);
+ connection_attempt_unref (attempt);
}
- else
- connection_attempt_unref (attempt);
return;
}
@@ -1592,7 +1607,6 @@ g_socket_client_connected_callback (GObj
{
ConnectionAttempt *attempt_entry = l->data;
g_cancellable_cancel (attempt_entry->cancellable);
- attempt_entry->data = NULL;
connection_attempt_unref (attempt_entry);
}
g_slist_free (data->connection_attempts);
@@ -1625,7 +1639,7 @@ g_socket_client_connected_callback (GObj
G_IO_ERROR, G_IO_ERROR_NOT_SUPPORTED,
_("Proxying over a non-TCP connection is not supported."));
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
}
else if (g_hash_table_contains (data->client->priv->app_proxies, protocol))
{
@@ -1652,7 +1666,7 @@ g_socket_client_connected_callback (GObj
_("Proxy protocol “%s” is not supported."),
protocol);
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
}
}
@@ -1661,7 +1675,7 @@ on_connection_attempt_timeout (gpointer
{
ConnectionAttempt *attempt = data;
- enumerator_next_async (attempt->data);
+ enumerator_next_async (attempt->data, TRUE);
g_clear_pointer (&attempt->timeout_source, g_source_unref);
return G_SOURCE_REMOVE;
@@ -1687,7 +1701,7 @@ g_socket_client_enumerator_callback (GOb
ConnectionAttempt *attempt;
GError *error = NULL;
- if (g_task_return_error_if_cancelled (data->task))
+ if (task_completed_or_cancelled (data->task))
{
g_object_unref (data->task);
return;
@@ -1698,7 +1712,10 @@ g_socket_client_enumerator_callback (GOb
if (address == NULL)
{
if (data->connection_attempts)
- return;
+ {
+ g_object_unref (data->task);
+ return;
+ }
g_socket_client_emit_event (data->client, G_SOCKET_CLIENT_COMPLETE, data->connectable, NULL);
if (!error)
@@ -1732,7 +1749,7 @@ g_socket_client_enumerator_callback (GOb
if (socket == NULL)
{
g_object_unref (address);
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
return;
}
@@ -1804,11 +1821,24 @@ g_socket_client_connect_async (GSocketCl
else
data->enumerator = g_socket_connectable_enumerate (connectable);
+ /* The flow and ownership here isn't quite obvious:
+ - The task starts an async attempt to connect.
+ - Each attempt holds a single ref on task.
+ - Each attempt may create new attempts by timing out (not a failure) so
+ there are multiple attempts happening in parallel.
+ - Upon failure an attempt will start a new attempt that steals its ref
+ until there are no more attempts left and it drops its ref.
+ - Upon success it will cancel all other attempts and continue on
+ to the rest of the connection (tls, proxies, etc) which do not
+ happen in parallel and at the very end drop its ref.
+ - Upon cancellation an attempt drops its ref.
+ */
+
data->task = g_task_new (client, cancellable, callback, user_data);
g_task_set_source_tag (data->task, g_socket_client_connect_async);
g_task_set_task_data (data->task, data, (GDestroyNotify)g_socket_client_async_connect_data_free);
- enumerator_next_async (data);
+ enumerator_next_async (data, FALSE);
}
/**
|
