Stefano Garzarella
2023-Nov-06 10:43 UTC
[PATCH net 1/4] vsock/virtio: remove socket from connected/bound list on shutdown
On Fri, Nov 03, 2023 at 06:55:48PM +0100, f.storniolo95 at gmail.com wrote:>From: Filippo Storniolo <f.storniolo95 at 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 at gmail.com> >Tested-by: Daan De Meyer <daan.j.demeyer at gmail.com> >Co-developed-by: Luigi Leonardi <luigi.leonardi at outlook.com> >Signed-off-by: Luigi Leonardi <luigi.leonardi at outlook.com> >Signed-off-by: Filippo Storniolo <f.storniolo95 at 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 at redhat.com>