netdev - [PATCH net] hv_sock: add locking in the open/close/release code paths

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 19 Oct 2017 03:33:14 +0000
From:   Dexuan Cui <decui@...rosoft.com>
To:     "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "KY Srinivasan" <kys@...rosoft.com>
CC:     "devel@...uxdriverproject.org" <devel@...uxdriverproject.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Vitaly Kuznetsov" <vkuznets@...hat.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Cathy Avery <cavery@...hat.com>,
        Rolf Neugebauer <rolf.neugebauer@...ker.com>,
        Marcelo Cerri <marcelo.cerri@...onical.com>,
        Jork Loeser <Jork.Loeser@...rosoft.com>
Subject: [PATCH net] hv_sock: add locking in the open/close/release code paths


Without the patch, when hvs_open_connection() hasn't completely established
a connection (e.g. it has changed sk->sk_state to SS_CONNECTED, but hasn't
inserted the sock into the connected queue), vsock_stream_connect() may see
the sk_state change and return the connection to the userspace, and next
when the userspace closes the connection quickly, hvs_release() may not see
the connection in the connected queue; finally hvs_open_connection()
inserts the connection into the queue, but we won't be able to purge the
connection for ever.

Signed-off-by: Dexuan Cui <decui@...rosoft.com>
Cc: K. Y. Srinivasan <kys@...rosoft.com>
Cc: Haiyang Zhang <haiyangz@...rosoft.com>
Cc: Stephen Hemminger <sthemmin@...rosoft.com>
Cc: Vitaly Kuznetsov <vkuznets@...hat.com>
Cc: Cathy Avery <cavery@...hat.com>
Cc: Rolf Neugebauer <rolf.neugebauer@...ker.com>
Cc: Marcelo Cerri <marcelo.cerri@...onical.com>
---

Please consider this for v4.14.

 net/vmw_vsock/hyperv_transport.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 14ed5a3..e21991f 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -310,11 +310,15 @@ static void hvs_close_connection(struct vmbus_channel *chan)
 	struct sock *sk = get_per_channel_state(chan);
 	struct vsock_sock *vsk = vsock_sk(sk);
 
+	lock_sock(sk);
+
 	sk->sk_state = SS_UNCONNECTED;
 	sock_set_flag(sk, SOCK_DONE);
 	vsk->peer_shutdown |= SEND_SHUTDOWN | RCV_SHUTDOWN;
 
 	sk->sk_state_change(sk);
+
+	release_sock(sk);
 }
 
 static void hvs_open_connection(struct vmbus_channel *chan)
@@ -344,6 +348,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
 	if (!sk)
 		return;
 
+	lock_sock(sk);
+
 	if ((conn_from_host && sk->sk_state != VSOCK_SS_LISTEN) ||
 	    (!conn_from_host && sk->sk_state != SS_CONNECTING))
 		goto out;
@@ -395,9 +401,7 @@ static void hvs_open_connection(struct vmbus_channel *chan)
 
 		vsock_insert_connected(vnew);
 
-		lock_sock(sk);
 		vsock_enqueue_accept(sk, new);
-		release_sock(sk);
 	} else {
 		sk->sk_state = SS_CONNECTED;
 		sk->sk_socket->state = SS_CONNECTED;
@@ -410,6 +414,8 @@ static void hvs_open_connection(struct vmbus_channel *chan)
 out:
 	/* Release refcnt obtained when we called vsock_find_bound_socket() */
 	sock_put(sk);
+
+	release_sock(sk);
 }
 
 static u32 hvs_get_local_cid(void)
@@ -476,13 +482,21 @@ static int hvs_shutdown(struct vsock_sock *vsk, int mode)
 
 static void hvs_release(struct vsock_sock *vsk)
 {
+	struct sock *sk = sk_vsock(vsk);
 	struct hvsock *hvs = vsk->trans;
-	struct vmbus_channel *chan = hvs->chan;
+	struct vmbus_channel *chan;
 
+	lock_sock(sk);
+
+	sk->sk_state = SS_DISCONNECTING;
+	vsock_remove_sock(vsk);
+
+	release_sock(sk);
+
+	chan = hvs->chan;
 	if (chan)
 		hvs_shutdown(vsk, RCV_SHUTDOWN | SEND_SHUTDOWN);
 
-	vsock_remove_sock(vsk);
 }
 
 static void hvs_destruct(struct vsock_sock *vsk)
-- 
2.7.4