| Message ID | 20210621062601.1473-1-longpeng2@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c7ff9cff70601ea19245d997bb977344663434c7 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2] vsock: notify server to shutdown when client has pending signal | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 1 maintainers not CCed: arseny.krasnov@kaspersky.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 97 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Mon, Jun 21, 2021 at 02:26:01PM +0800, Longpeng(Mike) wrote: >The client's sk_state will be set to TCP_ESTABLISHED if the server >replay the client's connect request. > >However, if the client has pending signal, its sk_state will be set >to TCP_CLOSE without notify the server, so the server will hold the >corrupt connection. > > client server > >1. sk_state=TCP_SYN_SENT | >2. call ->connect() | >3. wait reply | > | 4. sk_state=TCP_ESTABLISHED > | 5. insert to connected list > | 6. reply to the client >7. sk_state=TCP_ESTABLISHED | >8. insert to connected list | >9. *signal pending* <--------------------- the user kill client >10. sk_state=TCP_CLOSE | >client is exiting... | >11. call ->release() | > virtio_transport_close > if (!(sk->sk_state == TCP_ESTABLISHED || > sk->sk_state == TCP_CLOSING)) > return true; *return at here, the server cannot notice the connection is corrupt* > >So the client should notify the peer in this case. > >Cc: David S. Miller <davem@davemloft.net> >Cc: Jakub Kicinski <kuba@kernel.org> >Cc: Jorgen Hansen <jhansen@vmware.com> >Cc: Norbert Slusarek <nslusarek@gmx.net> >Cc: Andra Paraschiv <andraprs@amazon.com> >Cc: Colin Ian King <colin.king@canonical.com> >Cc: David Brazdil <dbrazdil@google.com> >Cc: Alexander Popov <alex.popov@linux.com> >Suggested-by: Stefano Garzarella <sgarzare@redhat.com> >Link: https://lkml.org/lkml/2021/5/17/418 >Signed-off-by: lixianming <lixianming5@huawei.com> >Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com> >--- > 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 92a72f0..ae11311 100644 >--- a/net/vmw_vsock/af_vsock.c >+++ b/net/vmw_vsock/af_vsock.c >@@ -1369,7 +1369,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr, > > if (signal_pending(current)) { > err = sock_intr_errno(timeout); >- sk->sk_state = TCP_CLOSE; >+ sk->sk_state = sk->sk_state == TCP_ESTABLISHED ? TCP_CLOSING : TCP_CLOSE; > sock->state = SS_UNCONNECTED; > vsock_transport_cancel_pkt(vsk); > goto out_wait; >-- >1.8.3.1 > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Mon, 21 Jun 2021 14:26:01 +0800 you wrote: > The client's sk_state will be set to TCP_ESTABLISHED if the server > replay the client's connect request. > > However, if the client has pending signal, its sk_state will be set > to TCP_CLOSE without notify the server, so the server will hold the > corrupt connection. > > [...] Here is the summary with links: - [v2] vsock: notify server to shutdown when client has pending signal https://git.kernel.org/netdev/net/c/c7ff9cff7060 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c index 92a72f0..ae11311 100644 --- a/net/vmw_vsock/af_vsock.c +++ b/net/vmw_vsock/af_vsock.c @@ -1369,7 +1369,7 @@ static int vsock_stream_connect(struct socket *sock, struct sockaddr *addr, if (signal_pending(current)) { err = sock_intr_errno(timeout); - sk->sk_state = TCP_CLOSE; + sk->sk_state = sk->sk_state == TCP_ESTABLISHED ? TCP_CLOSING : TCP_CLOSE; sock->state = SS_UNCONNECTED; vsock_transport_cancel_pkt(vsk); goto out_wait;
The client's sk_state will be set to TCP_ESTABLISHED if the server replay the client's connect request. However, if the client has pending signal, its sk_state will be set to TCP_CLOSE without notify the server, so the server will hold the corrupt connection. client server 1. sk_state=TCP_SYN_SENT | 2. call ->connect() | 3. wait reply | | 4. sk_state=TCP_ESTABLISHED | 5. insert to connected list | 6. reply to the client 7. sk_state=TCP_ESTABLISHED | 8. insert to connected list | 9. *signal pending* <--------------------- the user kill client 10. sk_state=TCP_CLOSE | client is exiting... | 11. call ->release() | virtio_transport_close if (!(sk->sk_state == TCP_ESTABLISHED || sk->sk_state == TCP_CLOSING)) return true; *return at here, the server cannot notice the connection is corrupt* So the client should notify the peer in this case. Cc: David S. Miller <davem@davemloft.net> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Jorgen Hansen <jhansen@vmware.com> Cc: Norbert Slusarek <nslusarek@gmx.net> Cc: Andra Paraschiv <andraprs@amazon.com> Cc: Colin Ian King <colin.king@canonical.com> Cc: David Brazdil <dbrazdil@google.com> Cc: Alexander Popov <alex.popov@linux.com> Suggested-by: Stefano Garzarella <sgarzare@redhat.com> Link: https://lkml.org/lkml/2021/5/17/418 Signed-off-by: lixianming <lixianming5@huawei.com> Signed-off-by: Longpeng(Mike) <longpeng2@huawei.com> --- net/vmw_vsock/af_vsock.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)