2023-05-11 12:12:10

by Zhuang Shengen

[permalink] [raw]
Subject: [PATCH net v2] vsock: avoid to close connected socket after the timeout

When client and server establish a connection through vsock,
the client send a request to the server to initiate the connection,
then start a timer to wait for the server's response. When the server's
RESPONSE message arrives, the timer also times out and exits. The
server's RESPONSE message is processed first, and the connection is
established. However, the client's timer also times out, the original
processing logic of the client is to directly set the state of this vsock
to CLOSE and return ETIMEDOUT. It will not notify the server when the port
is released, causing the server port remain.
when client's vsock_connect timeout,it should check sk state is
ESTABLISHED or not. if sk state is ESTABLISHED, it means the connection
is established, the client should not set the sk state to CLOSE

Note: I encountered this issue on kernel-4.18, which can be fixed by
this patch. Then I checked the latest code in the community
and found similar issue.

Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
Signed-off-by: Zhuang Shengen <[email protected]>
---
net/vmw_vsock/af_vsock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 413407bb646c..efb8a0937a13 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -1462,7 +1462,7 @@ static int vsock_connect(struct socket *sock, struct sockaddr *addr,
vsock_transport_cancel_pkt(vsk);
vsock_remove_connected(vsk);
goto out_wait;
- } else if (timeout == 0) {
+ } else if ((sk->sk_state != TCP_ESTABLISHED) && (timeout == 0)) {
err = -ETIMEDOUT;
sk->sk_state = TCP_CLOSE;
sock->state = SS_UNCONNECTED;
--
2.27.0



2023-05-11 13:08:26

by Stefano Garzarella

[permalink] [raw]
Subject: Re: [PATCH net v2] vsock: avoid to close connected socket after the timeout

On Thu, May 11, 2023 at 07:34:30PM +0800, Zhuang Shengen wrote:
>When client and server establish a connection through vsock,
>the client send a request to the server to initiate the connection,
>then start a timer to wait for the server's response. When the server's
>RESPONSE message arrives, the timer also times out and exits. The
>server's RESPONSE message is processed first, and the connection is
>established. However, the client's timer also times out, the original
>processing logic of the client is to directly set the state of this vsock
>to CLOSE and return ETIMEDOUT. It will not notify the server when the port
>is released, causing the server port remain.
>when client's vsock_connect timeout,it should check sk state is
>ESTABLISHED or not. if sk state is ESTABLISHED, it means the connection
>is established, the client should not set the sk state to CLOSE
>
>Note: I encountered this issue on kernel-4.18, which can be fixed by
>this patch. Then I checked the latest code in the community
>and found similar issue.
>
>Fixes: d021c344051a ("VSOCK: Introduce VM Sockets")
>Signed-off-by: Zhuang Shengen <[email protected]>
>---
> net/vmw_vsock/af_vsock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)

The patch LGTM:

Reviewed-by: Stefano Garzarella <[email protected]>

I noticed that the net maintainers are not in cc, if this patch isn't
queued in the next days, please resend it by cc'ing all the
maintainers [1]:

$ ./scripts/get_maintainer.pl \
20230510_zhuangshengen_vsock_bugfix_port_residue_in_server.mbx
Stefano Garzarella <[email protected]> (maintainer:VM SOCKETS (AF_VSOCK))
"David S. Miller" <[email protected]> (maintainer:NETWORKING [GENERAL])
Eric Dumazet <[email protected]> (maintainer:NETWORKING [GENERAL])
Jakub Kicinski <[email protected]> (maintainer:NETWORKING [GENERAL])
Paolo Abeni <[email protected]> (maintainer:NETWORKING [GENERAL])
[email protected] (open list:VM SOCKETS (AF_VSOCK))
[email protected] (open list:VM SOCKETS (AF_VSOCK))
[email protected] (open list)

Thanks,
Stefano

[1] https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#select-the-recipients-for-your-patch

>
>diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
>index 413407bb646c..efb8a0937a13 100644
>--- a/net/vmw_vsock/af_vsock.c
>+++ b/net/vmw_vsock/af_vsock.c
>@@ -1462,7 +1462,7 @@ static int vsock_connect(struct socket *sock, struct sockaddr *addr,
> vsock_transport_cancel_pkt(vsk);
> vsock_remove_connected(vsk);
> goto out_wait;
>- } else if (timeout == 0) {
>+ } else if ((sk->sk_state != TCP_ESTABLISHED) && (timeout == 0)) {
> err = -ETIMEDOUT;
> sk->sk_state = TCP_CLOSE;
> sock->state = SS_UNCONNECTED;
>--
>2.27.0
>


2023-05-12 09:22:29

by patchwork-bot+netdevbpf

[permalink] [raw]
Subject: Re: [PATCH net v2] vsock: avoid to close connected socket after the timeout

Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <[email protected]>:

On Thu, 11 May 2023 19:34:30 +0800 you wrote:
> When client and server establish a connection through vsock,
> the client send a request to the server to initiate the connection,
> then start a timer to wait for the server's response. When the server's
> RESPONSE message arrives, the timer also times out and exits. The
> server's RESPONSE message is processed first, and the connection is
> established. However, the client's timer also times out, the original
> processing logic of the client is to directly set the state of this vsock
> to CLOSE and return ETIMEDOUT. It will not notify the server when the port
> is released, causing the server port remain.
> when client's vsock_connect timeout,it should check sk state is
> ESTABLISHED or not. if sk state is ESTABLISHED, it means the connection
> is established, the client should not set the sk state to CLOSE
>
> [...]

Here is the summary with links:
- [net,v2] vsock: avoid to close connected socket after the timeout
https://git.kernel.org/netdev/net/c/6d4486efe9c6

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html