diff mbox series

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

Message ID 20180830155757.103694-2-gavingrant@protonmail.com
State New
Headers show
Series slirp: Propagate host TCP RST packet to the guest after socket disconnected | expand

Commit Message

Cameron Esfahani via Aug. 30, 2018, 3:57 p.m. UTC
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(-)

Comments

Samuel Thibault Aug. 30, 2018, 4:02 p.m. UTC | #1
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 Thibault Oct. 7, 2018, 5:52 p.m. UTC | #2
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
diff mbox series

Patch

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)));