[Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

[Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

[Gavin Grant]

[PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

Please bear with me, since this is my first correspondence with the qemu-devel
mailing list (or any public devel mailing list for that matter!).

I believe I have witnessed a discrepancy between SLIRP TCP and normal UNIX/Linux
TCP behaviour relating to the sending of RST packet(s) after a socket has been
closed gracefully by a host client application, then the application
subsequently exits. When the host socket is closed gracefully, it is in a
"half-closed" state. i.e. The socket can no longer be read from, but it can be
written to and will wait in FIN_WAIT_1 & FIN_WAIT_2 states until the remote peer
sends a FIN. This is fine. But when the application exits, the host socket is no
longer connected. I would expect subsequent slirp_send() calls to return an EPIPE
error, causing RST packet(s) to be sent to the server, causing it to close its
socket.

However, unlike soread() in 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799, sowrite()
does not call tcp_drop() for specific errno's, so the server socket will remain
open. sbappend() does not implement error handling, as it relies on errors to
be handled in soread().

My proposed patch attempts to address this by checking for closed & disconnected
socket status in soread(). Forgive me if my logic and proposal are foolish!

The application(s) that I observed this behaviour from were libiio test
programs (https://github.com/analogdevicesinc/libiio/tree/master/tests), which
connect to an iiod daemon process running on the server, to interface with the
Linux Industrial Input/Output (IIO) subsystem. I noticed that if iiod is running
on a QEMU server with SLIRP, then iiod does not notice when client applications
close their connection and exit. This can result in multiple threads open on the
server, with their sockets stuck in the CLOSE_WAIT state. Once several of these
threads exist, my QEMU system becomes extremely slow and unusable.

[Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

[Gavin Grant]

Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP
connection is abruptly closed via a RST packet, by checking for the ECONNRESET
errno. However it does not consider the case where the connection has been
half-closed by the host (FIN/ACK), then the host socket is disconnected. For
example, if the host application calls close() on the socket, then the
application exits.

In this case, the socket still exists due to the file descriptor in SLIRP, but
it is disconnected. recv() does not indicate an error since an orderly socket
close has previously occurred. The socket will then be stuck in FIN_WAIT_2,
until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST
to the peer and transition to the CLOSED state.

Signed-off-by: Gavin Grant <gavingrant@protonmail.com>
---
 slirp/socket.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 08fe98907d..543bd5feaf 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -204,12 +204,17 @@ soread(struct socket *so)
 			return 0;
 		else {
 			int err;
+			struct sockaddr addr;
 			socklen_t slen = sizeof err;
 
 			err = errno;
 			if (nn == 0) {
-				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
-					   &err, &slen);
+				if (getpeername(so->s, &addr, &slen) < 0) {
+					err = errno;
+				} else {
+					getsockopt(so->s, SOL_SOCKET, SO_ERROR,
+						&err, &slen);
+				}
 			}
 
 			DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));
-- 
2.11.0

Re: [Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

[Samuel Thibault]

Hello,

The principle seems sane, I'll have a look.

Thanks,
Samuel

Gavin Grant, le jeu. 30 août 2018 16:57:57 +0100, a ecrit:
> Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP
> connection is abruptly closed via a RST packet, by checking for the ECONNRESET
> errno. However it does not consider the case where the connection has been
> half-closed by the host (FIN/ACK), then the host socket is disconnected. For
> example, if the host application calls close() on the socket, then the
> application exits.
> 
> In this case, the socket still exists due to the file descriptor in SLIRP, but
> it is disconnected. recv() does not indicate an error since an orderly socket
> close has previously occurred. The socket will then be stuck in FIN_WAIT_2,
> until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST
> to the peer and transition to the CLOSED state.
> 
> Signed-off-by: Gavin Grant <gavingrant@protonmail.com>
> ---
>  slirp/socket.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/slirp/socket.c b/slirp/socket.c
> index 08fe98907d..543bd5feaf 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -204,12 +204,17 @@ soread(struct socket *so)
>  			return 0;
>  		else {
>  			int err;
> +			struct sockaddr addr;
>  			socklen_t slen = sizeof err;
>  
>  			err = errno;
>  			if (nn == 0) {
> -				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> -					   &err, &slen);
> +				if (getpeername(so->s, &addr, &slen) < 0) {
> +					err = errno;
> +				} else {
> +					getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> +						&err, &slen);
> +				}
>  			}
>  
>  			DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));
> -- 
> 2.11.0
> 

-- 
Samuel
***e trouve un .xls
***e passe un moment à se demander quelle version de xml c'est ça, le .xls
e: donc j'ai fait un file....
 -+- #sos - on n'a pas forcément les mêmes références que tout le monde -+-

Re: [Qemu-devel] [PATCH] slirp: Propagate host TCP RST packet to the guest after socket disconnected

[Samuel Thibault]

Hello,

Gavin Grant, le jeu. 30 août 2018 16:57:57 +0100, a ecrit:
> Commit 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 handled the case where the TCP
> connection is abruptly closed via a RST packet, by checking for the ECONNRESET
> errno. However it does not consider the case where the connection has been
> half-closed by the host (FIN/ACK), then the host socket is disconnected. For
> example, if the host application calls close() on the socket, then the
> application exits.
> 
> In this case, the socket still exists due to the file descriptor in SLIRP, but
> it is disconnected. recv() does not indicate an error since an orderly socket
> close has previously occurred. The socket will then be stuck in FIN_WAIT_2,
> until the peer sends FIN/ACK or a timeout occurs. Instead we can send a RST
> to the peer and transition to the CLOSED state.
> 
> Signed-off-by: Gavin Grant <gavingrant@protonmail.com>

I have fixed it a bit and pushed to my tree, thanks!

Samuel