diff mbox

[net] tcp: fix potential double free issue for fastopen_req

Message ID 20170301212948.96840-1-tracywwnj@gmail.com
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Wei Wang March 1, 2017, 9:29 p.m. UTC 
From: Wei Wang <weiwan@google.com>

tp->fastopen_req could potentially be double freed if a malicious
user does the following:
1. Enable TCP_FASTOPEN_CONNECT sockopt and do a connect() on the socket.
2. Call connect() with AF_UNSPEC to disconnect the socket.
3. Make this socket a listening socket by calling listen().
4. Accept incoming connections and generate child sockets. All child
   sockets will get a copy of the pointer of fastopen_req.
5. Call close() on all sockets. fastopen_req will get freed multiple
   times.

Fixes: 19f6d3f3c842 ("net/tcp-fastopen: Add new API support")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Wei Wang <weiwan@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/tcp.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

David Miller March 2, 2017, 10:06 p.m. UTC | #1 
From: Wei Wang <weiwan@google.com>
Date: Wed,  1 Mar 2017 13:29:48 -0800

> From: Wei Wang <weiwan@google.com>
> 
> tp->fastopen_req could potentially be double freed if a malicious
> user does the following:
> 1. Enable TCP_FASTOPEN_CONNECT sockopt and do a connect() on the socket.
> 2. Call connect() with AF_UNSPEC to disconnect the socket.
> 3. Make this socket a listening socket by calling listen().
> 4. Accept incoming connections and generate child sockets. All child
>    sockets will get a copy of the pointer of fastopen_req.
> 5. Call close() on all sockets. fastopen_req will get freed multiple
>    times.
> 
> Fixes: 19f6d3f3c842 ("net/tcp-fastopen: Add new API support")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied, and queued up for -stable.
diff mbox

Patch

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index da385ae997a3..cf4555581282 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1110,9 +1110,14 @@  static int tcp_sendmsg_fastopen(struct sock *sk, struct msghdr *msg,
 	flags = (msg->msg_flags & MSG_DONTWAIT) ? O_NONBLOCK : 0;
 	err = __inet_stream_connect(sk->sk_socket, msg->msg_name,
 				    msg->msg_namelen, flags, 1);
-	inet->defer_connect = 0;
-	*copied = tp->fastopen_req->copied;
-	tcp_free_fastopen_req(tp);
+	/* fastopen_req could already be freed in __inet_stream_connect
+	 * if the connection times out or gets rst
+	 */
+	if (tp->fastopen_req) {
+		*copied = tp->fastopen_req->copied;
+		tcp_free_fastopen_req(tp);
+		inet->defer_connect = 0;
+	}
 	return err;
 }
 
@@ -2318,6 +2323,10 @@  int tcp_disconnect(struct sock *sk, int flags)
 	memset(&tp->rx_opt, 0, sizeof(tp->rx_opt));
 	__sk_dst_reset(sk);
 
+	/* Clean up fastopen related fields */
+	tcp_free_fastopen_req(tp);
+	inet->defer_connect = 0;
+
 	WARN_ON(inet->inet_num && !icsk->icsk_bind_hash);
 
 	sk->sk_error_report(sk);