Re: Patch "net: virtio_vsock: Enhance connection semantics" has been added to the 4.9-stable tree

[Greg KH]

On Fri, Oct 02, 2020 at 07:18:20PM -0400, Sasha Levin wrote:
> This is a note to let you know that I've just added the patch titled
> 
>     net: virtio_vsock: Enhance connection semantics
> 
> to the 4.9-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      net-virtio_vsock-enhance-connection-semantics.patch
> and it can be found in the queue-4.9 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@xxxxxxxxxxxxxxx> know about it.
> 
> 
> 
> commit 298b295d45688ac79bebe2e87efa96711e2e0bad
> Author: Sebastien Boeuf <sebastien.boeuf@xxxxxxxxx>
> Date:   Fri Feb 14 12:48:01 2020 +0100
> 
>     net: virtio_vsock: Enhance connection semantics
>     
>     [ Upstream commit df12eb6d6cd920ab2f0e0a43cd6e1c23a05cea91 ]
>     
>     Whenever the vsock backend on the host sends a packet through the RX
>     queue, it expects an answer on the TX queue. Unfortunately, there is one
>     case where the host side will hang waiting for the answer and might
>     effectively never recover if no timeout mechanism was implemented.
>     
>     This issue happens when the guest side starts binding to the socket,
>     which insert a new bound socket into the list of already bound sockets.
>     At this time, we expect the guest to also start listening, which will
>     trigger the sk_state to move from TCP_CLOSE to TCP_LISTEN. The problem
>     occurs if the host side queued a RX packet and triggered an interrupt
>     right between the end of the binding process and the beginning of the
>     listening process. In this specific case, the function processing the
>     packet virtio_transport_recv_pkt() will find a bound socket, which means
>     it will hit the switch statement checking for the sk_state, but the
>     state won't be changed into TCP_LISTEN yet, which leads the code to pick
>     the default statement. This default statement will only free the buffer,
>     while it should also respond to the host side, by sending a packet on
>     its TX queue.
>     
>     In order to simply fix this unfortunate chain of events, it is important
>     that in case the default statement is entered, and because at this stage
>     we know the host side is waiting for an answer, we must send back a
>     packet containing the operation VIRTIO_VSOCK_OP_RST.
>     
>     One could say that a proper timeout mechanism on the host side will be
>     enough to avoid the backend to hang. But the point of this patch is to
>     ensure the normal use case will be provided with proper responsiveness
>     when it comes to establishing the connection.
>     
>     Signed-off-by: Sebastien Boeuf <sebastien.boeuf@xxxxxxxxx>
>     Signed-off-by: David S. Miller <davem@xxxxxxxxxxxxx>
>     Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> 
> diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
> index d64285afe68f3..f66bd0510f138 100644
> --- a/net/vmw_vsock/virtio_transport_common.c
> +++ b/net/vmw_vsock/virtio_transport_common.c
> @@ -986,6 +986,7 @@ void virtio_transport_recv_pkt(struct virtio_transport *t,
>  		virtio_transport_free_pkt(pkt);
>  		break;
>  	default:
> +		(void)virtio_transport_reset_no_sock(t, pkt);
>  		virtio_transport_free_pkt(pkt);
>  		break;
>  	}


Also breaks the build on 4.9, so dropping it.

greg k-h