Stefano Garzarella
2022-Feb-17 08:34 UTC
[PATCH] vsock: remove vsock from connected table when connect is interrupted by a signal
On Wed, Feb 16, 2022 at 08:14:59PM -0800, Jakub Kicinski wrote:>On Wed, 16 Feb 2022 17:11:22 +0100 Stefano Garzarella wrote: >> On Wed, Feb 16, 2022 at 08:32:22AM -0600, Seth Forshee wrote: >> >vsock_connect() expects that the socket could already be in the >> >TCP_ESTABLISHED state when the connecting task wakes up with a signal >> >pending. If this happens the socket will be in the connected table, and >> >it is not removed when the socket state is reset. In this situation it's >> >common for the process to retry connect(), and if the connection is >> >successful the socket will be added to the connected table a second >> >time, corrupting the list. >> > >> >Prevent this by calling vsock_remove_connected() if a signal is received >> >while waiting for a connection. This is harmless if the socket is not in >> >the connected table, and if it is in the table then removing it will >> >prevent list corruption from a double add. >> > >> >Signed-off-by: Seth Forshee <sforshee at digitalocean.com> >> >--- >> > net/vmw_vsock/af_vsock.c | 1 + >> > 1 file changed, 1 insertion(+) >> > >> >diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c >> >index 3235261f138d..38baeb189d4e 100644 >> >--- a/net/vmw_vsock/af_vsock.c >> >+++ b/net/vmw_vsock/af_vsock.c >> >@@ -1401,6 +1401,7 @@ static int vsock_connect(struct socket *sock, struct sockaddr *addr, >> > sk->sk_state = sk->sk_state == TCP_ESTABLISHED ? TCP_CLOSING : TCP_CLOSE; >> > sock->state = SS_UNCONNECTED; >> > vsock_transport_cancel_pkt(vsk); >> >+ vsock_remove_connected(vsk); >> > goto out_wait; >> > } else if (timeout == 0) { >> > err = -ETIMEDOUT; >> >> Thanks for this fix! The patch LGTM: >> >> Reviewed-by: Stefano Garzarella <sgarzare at redhat.com> >> >> >> @Dave, @Jakub, since we need this also in stable branches, I was going >> to suggest adding a Fixes tag, but I'm a little confused: the issue >> seems to have always been there, so from commit d021c344051a ("VSOCK: >> Introduce VM Sockets"), but to use vsock_remove_connected() as we are >> using in this patch, we really need commit d5afa82c977e ("vsock: correct >> removal of socket from the list"). >> >> Commit d5afa82c977e was introduces in v5.3 and it was backported in >> v4.19 and v4.14, but not in v4.9. >> So if we want to backport this patch also for v4.9, I think we need >> commit d5afa82c977e as well. > >The fixes tag sounds good. Dunno what's the best way to handle this >case. We can add a mention of the dependency to the patch description. >Personally I'd keep things simple, add the Fixes tag and keep an eye >on the backports, if 4.9 doesn't get it - email Greg and explain. >Okay, I'll keep an eye on this patch for 4.9. @Seth, can you send a v2 mentioning the dependency with commit d5afa82c977e ("vsock: correct removal of socket from the list") and adding the following fixes tag? Fixes: d021c344051a ("VSOCK: Introduce VM Sockets") Thanks, Stefano