* [PATCH net] hv_sock: add locking in the open/close/release code paths
@ 2017-10-19 3:33 ` Dexuan Cui
0 siblings, 0 replies; 3+ messages in thread
From: Dexuan Cui @ 2017-10-19 3:33 UTC (permalink / raw)
To: David S. Miller, netdev, Stephen Hemminger, KY Srinivasan
Cc: devel, linux-kernel, Vitaly Kuznetsov, Haiyang Zhang,
Cathy Avery, Rolf Neugebauer, Marcelo Cerri, Jork Loeser
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@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Cathy Avery <cavery@redhat.com>
Cc: Rolf Neugebauer <rolf.neugebauer@docker.com>
Cc: Marcelo Cerri <marcelo.cerri@canonical.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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH net] hv_sock: add locking in the open/close/release code paths
@ 2017-10-19 3:33 ` Dexuan Cui
0 siblings, 0 replies; 3+ messages in thread
From: Dexuan Cui @ 2017-10-19 3:33 UTC (permalink / raw)
To: David S. Miller, netdev, Stephen Hemminger, KY Srinivasan
Cc: Jork Loeser, Rolf Neugebauer, Haiyang Zhang, linux-kernel,
Marcelo Cerri, devel, Vitaly Kuznetsov
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@microsoft.com>
Cc: K. Y. Srinivasan <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Cathy Avery <cavery@redhat.com>
Cc: Rolf Neugebauer <rolf.neugebauer@docker.com>
Cc: Marcelo Cerri <marcelo.cerri@canonical.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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] hv_sock: add locking in the open/close/release code paths
2017-10-19 3:33 ` Dexuan Cui
(?)
@ 2017-10-21 1:21 ` David Miller
-1 siblings, 0 replies; 3+ messages in thread
From: David Miller @ 2017-10-21 1:21 UTC (permalink / raw)
To: decui
Cc: netdev, sthemmin, kys, devel, linux-kernel, vkuznets, haiyangz,
cavery, rolf.neugebauer, marcelo.cerri, Jork.Loeser
From: Dexuan Cui <decui@microsoft.com>
Date: Thu, 19 Oct 2017 03:33:14 +0000
>
> 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@microsoft.com>
Applied.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-10-21 1:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-19 3:33 [PATCH net] hv_sock: add locking in the open/close/release code paths Dexuan Cui
2017-10-19 3:33 ` Dexuan Cui
2017-10-21 1:21 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.