| Message ID | 20231103175551.41025-2-f.storniolo95@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3a5cc90a4d1756072619fe511d07621bdef7f120 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | vsock: fix server prevents clients from reconnecting | expand |
On Fri, Nov 03, 2023 at 06:55:48PM +0100, f.storniolo95@gmail.com wrote: >From: Filippo Storniolo <f.storniolo95@gmail.com> > >If the same remote peer, using the same port, tries to connect >to a server on a listening port more than once, the server will >reject the connection, causing a "connection reset by peer" >error on the remote peer. This is due to the presence of a >dangling socket from a previous connection in both the connected >and bound socket lists. >The inconsistency of the above lists only occurs when the remote >peer disconnects and the server remains active. > >This bug does not occur when the server socket is closed: >virtio_transport_release() will eventually schedule a call to >virtio_transport_do_close() and the latter will remove the socket >from the bound and connected socket lists and clear the sk_buff. > >However, virtio_transport_do_close() will only perform the above >actions if it has been scheduled, and this will not happen >if the server is processing the shutdown message from a remote peer. > >To fix this, introduce a call to vsock_remove_sock() >when the server is handling a client disconnect. >This is to remove the socket from the bound and connected socket >lists without clearing the sk_buff. > >Fixes: 06a8fc78367d ("VSOCK: Introduce virtio_vsock_common.ko") >Reported-by: Daan De Meyer <daan.j.demeyer@gmail.com> >Tested-by: Daan De Meyer <daan.j.demeyer@gmail.com> >Co-developed-by: Luigi Leonardi <luigi.leonardi@outlook.com> >Signed-off-by: Luigi Leonardi <luigi.leonardi@outlook.com> >Signed-off-by: Filippo Storniolo <f.storniolo95@gmail.com> >--- > net/vmw_vsock/virtio_transport_common.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > >diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c >index e22c81435ef7..4c595dd1fd64 100644 >--- a/net/vmw_vsock/virtio_transport_common.c >+++ b/net/vmw_vsock/virtio_transport_common.c >@@ -1369,11 +1369,17 @@ virtio_transport_recv_connected(struct sock *sk, > vsk->peer_shutdown |= RCV_SHUTDOWN; > if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SHUTDOWN_SEND) > vsk->peer_shutdown |= SEND_SHUTDOWN; >- if (vsk->peer_shutdown == SHUTDOWN_MASK && >- vsock_stream_has_data(vsk) <= 0 && >- !sock_flag(sk, SOCK_DONE)) { >- (void)virtio_transport_reset(vsk, NULL); >- virtio_transport_do_close(vsk, true); >+ if (vsk->peer_shutdown == SHUTDOWN_MASK) { >+ if (vsock_stream_has_data(vsk) <= 0 && !sock_flag(sk, SOCK_DONE)) { >+ (void)virtio_transport_reset(vsk, NULL); >+ virtio_transport_do_close(vsk, true); >+ } >+ /* Remove this socket anyway because the remote peer sent >+ * the shutdown. This way a new connection will succeed >+ * if the remote peer uses the same source port, >+ * even if the old socket is still unreleased, but now disconnected. >+ */ >+ vsock_remove_sock(vsk); > } > if (le32_to_cpu(virtio_vsock_hdr(skb)->flags)) > sk->sk_state_change(sk); >-- >2.41.0 > Thanks for fixing this issue! LGTM. Just to inform other maintainers as well. Daan reported this issue to me at DevConf.cz, I shared it with Filippo and Luigi who analyzed and solved it. Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c index e22c81435ef7..4c595dd1fd64 100644 --- a/net/vmw_vsock/virtio_transport_common.c +++ b/net/vmw_vsock/virtio_transport_common.c @@ -1369,11 +1369,17 @@ virtio_transport_recv_connected(struct sock *sk, vsk->peer_shutdown |= RCV_SHUTDOWN; if (le32_to_cpu(hdr->flags) & VIRTIO_VSOCK_SHUTDOWN_SEND) vsk->peer_shutdown |= SEND_SHUTDOWN; - if (vsk->peer_shutdown == SHUTDOWN_MASK && - vsock_stream_has_data(vsk) <= 0 && - !sock_flag(sk, SOCK_DONE)) { - (void)virtio_transport_reset(vsk, NULL); - virtio_transport_do_close(vsk, true); + if (vsk->peer_shutdown == SHUTDOWN_MASK) { + if (vsock_stream_has_data(vsk) <= 0 && !sock_flag(sk, SOCK_DONE)) { + (void)virtio_transport_reset(vsk, NULL); + virtio_transport_do_close(vsk, true); + } + /* Remove this socket anyway because the remote peer sent + * the shutdown. This way a new connection will succeed + * if the remote peer uses the same source port, + * even if the old socket is still unreleased, but now disconnected. + */ + vsock_remove_sock(vsk); } if (le32_to_cpu(virtio_vsock_hdr(skb)->flags)) sk->sk_state_change(sk);