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
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
|
From MAILER-DAEMON Wed Oct 14 16:34:37 2020
From: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
To: netdev@vger.kernel.org
Cc: Gerrit Renker <gerrit@erg.abdn.ac.uk>, "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Thadeu Lima de Souza Cascardo <cascardo@canonical.com>, "Gustavo A. R. Silva" <gustavoars@kernel.org>, "Alexander A. Klimov" <grandmaster@al2klimov.de>, Kees Cook <keescook@chromium.org>, Eric Dumazet <edumazet@google.com>, Alexey Kodanev <alexey.kodanev@oracle.com>, dccp@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] dccp: ccid: move timers to struct dccp_sock
Date: Tue, 13 Oct 2020 19:18:48 +0200
Message-Id: <20201013171849.236025-2-kleber.souza@canonical.com>
In-Reply-To: <20201013171849.236025-1-kleber.souza@canonical.com>
References: <20201013171849.236025-1-kleber.souza@canonical.com>
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
When dccps_hc_tx_ccid is freed, ccid timers may still trigger. The reason
del_timer_sync can't be used is because this relies on keeping a reference
to struct sock. But as we keep a pointer to dccps_hc_tx_ccid and free that
during disconnect, the timer should really belong to struct dccp_sock.
This addresses CVE-2020-16119.
Fixes: 839a6094140a (net: dccp: Convert timers to use timer_setup())
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Acked-bd: Richard Sailer <richard_siegfried@systemli.org>
---
include/linux/dccp.h | 2 ++
net/dccp/ccids/ccid2.c | 32 +++++++++++++++++++-------------
net/dccp/ccids/ccid3.c | 30 ++++++++++++++++++++----------
3 files changed, 41 insertions(+), 23 deletions(-)
diff --git a/include/linux/dccp.h b/include/linux/dccp.h
index 07e547c02fd8..504afa1a4be6 100644
--- a/include/linux/dccp.h
+++ b/include/linux/dccp.h
@@ -259,6 +259,7 @@ struct dccp_ackvec;
* @dccps_sync_scheduled - flag which signals "send out-of-band message soon"
* @dccps_xmitlet - tasklet scheduled by the TX CCID to dequeue data packets
* @dccps_xmit_timer - used by the TX CCID to delay sending (rate-based pacing)
+ * @dccps_ccid_timer - used by the CCIDs
* @dccps_syn_rtt - RTT sample from Request/Response exchange (in usecs)
*/
struct dccp_sock {
@@ -303,6 +304,7 @@ struct dccp_sock {
__u8 dccps_sync_scheduled:1;
struct tasklet_struct dccps_xmitlet;
struct timer_list dccps_xmit_timer;
+ struct timer_list dccps_ccid_timer;
};
static inline struct dccp_sock *dccp_sk(const struct sock *sk)
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index 3da1f77bd039..dbca1f1e2449 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -126,21 +126,26 @@ static void dccp_tasklet_schedule(struct sock *sk)
static void ccid2_hc_tx_rto_expire(struct timer_list *t)
{
- struct ccid2_hc_tx_sock *hc = from_timer(hc, t, tx_rtotimer);
- struct sock *sk = hc->sk;
- const bool sender_was_blocked = ccid2_cwnd_network_limited(hc);
+ struct dccp_sock *dp = from_timer(dp, t, dccps_ccid_timer);
+ struct sock *sk = (struct sock *)dp;
+ struct ccid2_hc_tx_sock *hc;
+ bool sender_was_blocked;
bh_lock_sock(sk);
+
+ if (inet_sk_state_load(sk) == DCCP_CLOSED)
+ goto out;
+
+ hc = ccid_priv(dp->dccps_hc_tx_ccid);
+ sender_was_blocked = ccid2_cwnd_network_limited(hc);
+
if (sock_owned_by_user(sk)) {
- sk_reset_timer(sk, &hc->tx_rtotimer, jiffies + HZ / 5);
+ sk_reset_timer(sk, &dp->dccps_ccid_timer, jiffies + HZ / 5);
goto out;
}
ccid2_pr_debug("RTO_EXPIRE\n");
- if (sk->sk_state == DCCP_CLOSED)
- goto out;
-
/* back-off timer */
hc->tx_rto <<= 1;
if (hc->tx_rto > DCCP_RTO_MAX)
@@ -166,7 +171,7 @@ static void ccid2_hc_tx_rto_expire(struct timer_list *t)
if (sender_was_blocked)
dccp_tasklet_schedule(sk);
/* restart backed-off timer */
- sk_reset_timer(sk, &hc->tx_rtotimer, jiffies + hc->tx_rto);
+ sk_reset_timer(sk, &dp->dccps_ccid_timer, jiffies + hc->tx_rto);
out:
bh_unlock_sock(sk);
sock_put(sk);
@@ -330,7 +335,7 @@ static void ccid2_hc_tx_packet_sent(struct sock *sk, unsigned int len)
}
#endif
- sk_reset_timer(sk, &hc->tx_rtotimer, jiffies + hc->tx_rto);
+ sk_reset_timer(sk, &dp->dccps_ccid_timer, jiffies + hc->tx_rto);
#ifdef CONFIG_IP_DCCP_CCID2_DEBUG
do {
@@ -700,9 +705,9 @@ static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
/* restart RTO timer if not all outstanding data has been acked */
if (hc->tx_pipe == 0)
- sk_stop_timer(sk, &hc->tx_rtotimer);
+ sk_stop_timer(sk, &dp->dccps_ccid_timer);
else
- sk_reset_timer(sk, &hc->tx_rtotimer, jiffies + hc->tx_rto);
+ sk_reset_timer(sk, &dp->dccps_ccid_timer, jiffies + hc->tx_rto);
done:
/* check if incoming Acks allow pending packets to be sent */
if (sender_was_blocked && !ccid2_cwnd_network_limited(hc))
@@ -737,17 +742,18 @@ static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk)
hc->tx_last_cong = hc->tx_lsndtime = hc->tx_cwnd_stamp = ccid2_jiffies32;
hc->tx_cwnd_used = 0;
hc->sk = sk;
- timer_setup(&hc->tx_rtotimer, ccid2_hc_tx_rto_expire, 0);
+ timer_setup(&dp->dccps_ccid_timer, ccid2_hc_tx_rto_expire, 0);
INIT_LIST_HEAD(&hc->tx_av_chunks);
return 0;
}
static void ccid2_hc_tx_exit(struct sock *sk)
{
+ struct dccp_sock *dp = dccp_sk(sk);
struct ccid2_hc_tx_sock *hc = ccid2_hc_tx_sk(sk);
int i;
- sk_stop_timer(sk, &hc->tx_rtotimer);
+ sk_stop_timer(sk, &dp->dccps_ccid_timer);
for (i = 0; i < hc->tx_seqbufc; i++)
kfree(hc->tx_seqbuf[i]);
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index b9ee1a4a8955..685f4d046c0d 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -184,17 +184,24 @@ static inline void ccid3_hc_tx_update_win_count(struct ccid3_hc_tx_sock *hc,
static void ccid3_hc_tx_no_feedback_timer(struct timer_list *t)
{
- struct ccid3_hc_tx_sock *hc = from_timer(hc, t, tx_no_feedback_timer);
- struct sock *sk = hc->sk;
+ struct dccp_sock *dp = from_timer(dp, t, dccps_ccid_timer);
+ struct ccid3_hc_tx_sock *hc;
+ struct sock *sk = (struct sock *)dp;
unsigned long t_nfb = USEC_PER_SEC / 5;
bh_lock_sock(sk);
+
+ if (inet_sk_state_load(sk) == DCCP_CLOSED)
+ goto out;
+
if (sock_owned_by_user(sk)) {
/* Try again later. */
/* XXX: set some sensible MIB */
goto restart_timer;
}
+ hc = ccid_priv(dp->dccps_hc_tx_ccid);
+
ccid3_pr_debug("%s(%p, state=%s) - entry\n", dccp_role(sk), sk,
ccid3_tx_state_name(hc->tx_state));
@@ -250,8 +257,8 @@ static void ccid3_hc_tx_no_feedback_timer(struct timer_list *t)
t_nfb = max(hc->tx_t_rto, 2 * hc->tx_t_ipi);
restart_timer:
- sk_reset_timer(sk, &hc->tx_no_feedback_timer,
- jiffies + usecs_to_jiffies(t_nfb));
+ sk_reset_timer(sk, &dp->dccps_ccid_timer,
+ jiffies + usecs_to_jiffies(t_nfb));
out:
bh_unlock_sock(sk);
sock_put(sk);
@@ -280,7 +287,7 @@ static int ccid3_hc_tx_send_packet(struct sock *sk, struct sk_buff *skb)
return -EBADMSG;
if (hc->tx_state == TFRC_SSTATE_NO_SENT) {
- sk_reset_timer(sk, &hc->tx_no_feedback_timer, (jiffies +
+ sk_reset_timer(sk, &dp->dccps_ccid_timer, (jiffies +
usecs_to_jiffies(TFRC_INITIAL_TIMEOUT)));
hc->tx_last_win_count = 0;
hc->tx_t_last_win_count = now;
@@ -354,6 +361,7 @@ static void ccid3_hc_tx_packet_sent(struct sock *sk, unsigned int len)
static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
{
struct ccid3_hc_tx_sock *hc = ccid3_hc_tx_sk(sk);
+ struct dccp_sock *dp = dccp_sk(sk);
struct tfrc_tx_hist_entry *acked;
ktime_t now;
unsigned long t_nfb;
@@ -420,7 +428,7 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
(unsigned int)(hc->tx_x >> 6));
/* unschedule no feedback timer */
- sk_stop_timer(sk, &hc->tx_no_feedback_timer);
+ sk_stop_timer(sk, &dp->dccps_ccid_timer);
/*
* As we have calculated new ipi, delta, t_nom it is possible
@@ -445,8 +453,8 @@ static void ccid3_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
"expire in %lu jiffies (%luus)\n",
dccp_role(sk), sk, usecs_to_jiffies(t_nfb), t_nfb);
- sk_reset_timer(sk, &hc->tx_no_feedback_timer,
- jiffies + usecs_to_jiffies(t_nfb));
+ sk_reset_timer(sk, &dp->dccps_ccid_timer,
+ jiffies + usecs_to_jiffies(t_nfb));
}
static int ccid3_hc_tx_parse_options(struct sock *sk, u8 packet_type,
@@ -488,21 +496,23 @@ static int ccid3_hc_tx_parse_options(struct sock *sk, u8 packet_type,
static int ccid3_hc_tx_init(struct ccid *ccid, struct sock *sk)
{
+ struct dccp_sock *dp = dccp_sk(sk);
struct ccid3_hc_tx_sock *hc = ccid_priv(ccid);
hc->tx_state = TFRC_SSTATE_NO_SENT;
hc->tx_hist = NULL;
hc->sk = sk;
- timer_setup(&hc->tx_no_feedback_timer,
+ timer_setup(&dp->dccps_ccid_timer,
ccid3_hc_tx_no_feedback_timer, 0);
return 0;
}
static void ccid3_hc_tx_exit(struct sock *sk)
{
+ struct dccp_sock *dp = dccp_sk(sk);
struct ccid3_hc_tx_sock *hc = ccid3_hc_tx_sk(sk);
- sk_stop_timer(sk, &hc->tx_no_feedback_timer);
+ sk_stop_timer(sk, &dp->dccps_ccid_timer);
tfrc_tx_hist_purge(&hc->tx_hist);
}
--
2.25.1
From MAILER-DAEMON Wed Oct 14 16:34:37 2020
From: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
To: netdev@vger.kernel.org
Cc: Gerrit Renker <gerrit@erg.abdn.ac.uk>, "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, Thadeu Lima de Souza Cascardo <cascardo@canonical.com>, "Gustavo A. R. Silva" <gustavoars@kernel.org>, "Alexander A. Klimov" <grandmaster@al2klimov.de>, Kees Cook <keescook@chromium.org>, Eric Dumazet <edumazet@google.com>, Alexey Kodanev <alexey.kodanev@oracle.com>, dccp@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] Revert "dccp: don't free ccid2_hc_tx_sock struct in dccp_disconnect()"
Date: Tue, 13 Oct 2020 19:18:49 +0200
Message-Id: <20201013171849.236025-3-kleber.souza@canonical.com>
In-Reply-To: <20201013171849.236025-1-kleber.souza@canonical.com>
References: <20201013171849.236025-1-kleber.souza@canonical.com>
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
From: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
This reverts commit 2677d20677314101293e6da0094ede7b5526d2b1.
This fixes an issue that after disconnect, dccps_hc_tx_ccid will still be
kept, allowing the socket to be reused as a listener socket, and the cloned
socket will free its dccps_hc_tx_ccid, leading to a later use after free,
when the listener socket is closed.
This addresses CVE-2020-16119.
Fixes: 2677d2067731 (dccp: don't free ccid2_hc_tx_sock struct in dccp_disconnect())
Reported-by: Hadar Manor
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
Acked-by: Richard Sailer <richard_siegfried@systemli.org>
---
net/dccp/proto.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index 6d705d90c614..359e848dba6c 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -279,7 +279,9 @@ int dccp_disconnect(struct sock *sk, int flags)
dccp_clear_xmit_timers(sk);
ccid_hc_rx_delete(dp->dccps_hc_rx_ccid, sk);
+ ccid_hc_tx_delete(dp->dccps_hc_tx_ccid, sk);
dp->dccps_hc_rx_ccid = NULL;
+ dp->dccps_hc_tx_ccid = NULL;
__skb_queue_purge(&sk->sk_receive_queue);
__skb_queue_purge(&sk->sk_write_queue);
--
2.25.1
|
