diff mbox

[2/3] slirp: Propagate host TCP RST to the guest.

Message ID 1c0611a61621b2069df831b48b937fa49cc264bd.1459896208.git.steven@steven676.net (mailing list archive)
State New, archived
Headers show

Commit Message

Steven Luo April 6, 2016, 12:14 a.m. UTC
When the host aborts (RST) it's side of a TCP connection we need to
propagate that RST to the guest. The current code can leave such guest
connections dangling forever. Spotted by Jason Wessel.

[steven@steven676.net: coding style adjustments]
Signed-off-by: Steven Luo <steven+qemu@steven676.net>
---
Edgar proposed this patch many years ago:

https://lists.gnu.org/archive/html/qemu-devel/2008-06/msg00383.html

It doesn't appear that it was ever merged.  (It's the top Google result
for "QEMU slirp RST".)  I've been unable to test the specific case it
addresses (an established connection interrupted by RST), but the
discussion from 2008 seems to imply it worked for the person reporting
the problem then, and my next patch builds on this one.

As this patch isn't my work and did not come with a Signed-off-by line,
I'm not entirely clear on how I should handle that.

 slirp/socket.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Paolo Bonzini April 6, 2016, 8:28 a.m. UTC | #1
On 06/04/2016 02:14, steven@steven676.net wrote:
> As this patch isn't my work and did not come with a Signed-off-by line,
> I'm not entirely clear on how I should handle that.

Signed-off-by lines weren't in consistent use at the time, so I think
it's okay to leave it out.

Paolo
Edgar E. Iglesias April 6, 2016, 8:36 a.m. UTC | #2
On Tue, Apr 05, 2016 at 05:14:31PM -0700, steven@steven676.net wrote:
> When the host aborts (RST) it's side of a TCP connection we need to
> propagate that RST to the guest. The current code can leave such guest
> connections dangling forever. Spotted by Jason Wessel.
> 
> [steven@steven676.net: coding style adjustments]
> Signed-off-by: Steven Luo <steven+qemu@steven676.net>
> ---
> Edgar proposed this patch many years ago:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2008-06/msg00383.html
> 
> It doesn't appear that it was ever merged.  (It's the top Google result
> for "QEMU slirp RST".)  I've been unable to test the specific case it
> addresses (an established connection interrupted by RST), but the
> discussion from 2008 seems to imply it worked for the person reporting
> the problem then, and my next patch builds on this one.
> 
> As this patch isn't my work and did not come with a Signed-off-by line,
> I'm not entirely clear on how I should handle that.

Hi Steven,

I don't mind to leave it as is but you could also use the --author
argument to git commit to keep the authorship as edgar.iglesias@gmail.com

BTW, I had totally forgotten about this... thanks for picking it up!

Best regards,
Edgar


> 
>  slirp/socket.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/slirp/socket.c b/slirp/socket.c
> index b836c42..4372ec2 100644
> --- a/slirp/socket.c
> +++ b/slirp/socket.c
> @@ -176,9 +176,24 @@ soread(struct socket *so)
>  		if (nn < 0 && (errno == EINTR || errno == EAGAIN))
>  			return 0;
>  		else {
> +			int err;
> +			socklen_t slen = sizeof err;
> +
> +			err = errno;
> +			if (nn == 0) {
> +				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> +					   &err, &slen);
> +			}
> +
>  			DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));
>  			sofcantrcvmore(so);
> -			tcp_sockclosed(sototcpcb(so));
> +
> +			if (err == ECONNRESET
> +			    || err == ENOTCONN || err == EPIPE) {
> +				tcp_drop(sototcpcb(so), err);
> +			} else {
> +				tcp_sockclosed(sototcpcb(so));
> +			}
>  			return -1;
>  		}
>  	}
> -- 
> 2.1.4
>
Steven Luo April 6, 2016, 2:59 p.m. UTC | #3
On April 6, 2016 1:36:01 AM PDT, "Edgar E. Iglesias" <edgar.iglesias@xilinx.com> wrote:
>Hi Steven,
>
>I don't mind to leave it as is but you could also use the --author
>argument to git commit to keep the authorship as
>edgar.iglesias@gmail.com

It looks like an intermediate mail server mangled the From addresses somewhere along the way -- I have it as edgar.iglesias@axis.com in my local tree.  My apologies -- I'll fix and resend later.


-Steven Luo
diff mbox

Patch

diff --git a/slirp/socket.c b/slirp/socket.c
index b836c42..4372ec2 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -176,9 +176,24 @@  soread(struct socket *so)
 		if (nn < 0 && (errno == EINTR || errno == EAGAIN))
 			return 0;
 		else {
+			int err;
+			socklen_t slen = sizeof err;
+
+			err = errno;
+			if (nn == 0) {
+				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
+					   &err, &slen);
+			}
+
 			DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));
 			sofcantrcvmore(so);
-			tcp_sockclosed(sototcpcb(so));
+
+			if (err == ECONNRESET
+			    || err == ENOTCONN || err == EPIPE) {
+				tcp_drop(sototcpcb(so), err);
+			} else {
+				tcp_sockclosed(sototcpcb(so));
+			}
 			return -1;
 		}
 	}