[Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Jason Wessel]

[-- Attachment #1: Type: text/plain, Size: 2755 bytes --]


When using slirp networking with a redirected tcp socket, the qemu guest
os does not receive RST packets when a redirected, accepted socket goes
into the FIN_WAIT_2 status.  Presently slirp sends ACKs instead of RST
packets, which means the guest os application socket writes do not fail
event after the client has terminated the socket.

Here is a simple way to demonstrate the problem.

* Start qemu with user mode networking plus:
     -redir tcp:4441::4441

* Assuming you booted a linux guest os you could run:
     cat /dev/zero | nc -p 4441 -l

* On the host run the following command and you
  must hit control-c after about 1 second
     nc localhost 4441


If you were to TCP dump the connection in the guest OS you would see
after killing the "nc" on the host computer that slirp keep acking the
packets, even though no client application is there.

14:55:38.385310 IP 10.0.2.15.4441 > 10.0.2.2.37227: P
8884509:8885077(568) ack 2 win 5840
14:55:38.385310 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
14:55:38.589613 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
14:55:38.589613 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
14:55:38.997437 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
14:55:38.997653 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
14:55:39.813522 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
14:55:39.813758 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
14:55:41.445562 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
14:55:41.445769 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0


The correct behavior should be to send an RST and not an ACK.  There
might be several ways to correct this problem.  The attached patch is
one possible way to implement the RFC compliant behavior.  With the
patch, the tcp dump starts to look like:

15:04:34.567350 IP 10.0.2.15.4441 > 10.0.2.2.58510: P
2101533:2102993(1460) ack 1 win 5840
15:04:34.567350 IP 10.0.2.2.58510 > 10.0.2.15.4441: . ack 2102993 win 5840
15:04:34.570718 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2102993
win 5840
15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: .
2102993:2104453(1460) ack 1 win 5840
15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2104453
win 4380
15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: P
2104453:2105345(892) ack 1 win 5840
15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2105345
win 3488
15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: . ack 2 win 5840
15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: . ack 2 win 5840
15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: R
12032003:12032003(0) win 3488

Also with the patch, the SIG_PIPE handlers start to work correctly in
the guest OS.

Thanks,
Jason.

[-- Attachment #2: tcp_rst_fin_wait_2.patch --]
[-- Type: text/x-diff, Size: 947 bytes --]

From: Jason Wessel <jason.wessel@windriver.com>
Subject: [PATCH] slirp: Fix broken RST response to a slirp redirect socket

When using slirp networking with a redirected tcp socket, the qemu
guest os does not receive RST packets when a redirected, accepted
socket goes into the FIN_WAIT_2 status.  Presently slirp sends ACKs
instead of RST packets, which means the guest os application socket
writes do not fail event after the client has terminated the socket.

This patch changes the behavior to correctly send RST packets instead
of ACKS.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 slirp/tcp_input.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -432,7 +432,7 @@ findso:
 	tp = sototcpcb(so);
 
 	/* XXX Should never fail */
-	if (tp == 0)
+	if (tp == 0 || tp->t_state == TCPS_FIN_WAIT_2)
 		goto dropwithreset;
 	if (tp->t_state == TCPS_CLOSED)
 		goto drop;

Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Edgar E. Iglesias]

On Wed, Jun 11, 2008 at 12:21:45PM -0500, Jason Wessel wrote:
> 
> When using slirp networking with a redirected tcp socket, the qemu guest
> os does not receive RST packets when a redirected, accepted socket goes
> into the FIN_WAIT_2 status.  Presently slirp sends ACKs instead of RST
> packets, which means the guest os application socket writes do not fail
> event after the client has terminated the socket.
> 
> Here is a simple way to demonstrate the problem.
> 
> * Start qemu with user mode networking plus:
>      -redir tcp:4441::4441
> 
> * Assuming you booted a linux guest os you could run:
>      cat /dev/zero | nc -p 4441 -l
> 
> * On the host run the following command and you
>   must hit control-c after about 1 second
>      nc localhost 4441

Hello Jason,

IIRC connections in FIN_WAIT_2 can continue to receive data.

If I might take a wild guess at whats going on:
The host closed the receiving socket when you ctrl-c nc. That socket still has
data in it's rcvbuf so the stack aborts the connection and sends a RST. The
slirp code should now see a -1 on it's next write to that socket and an errno
ECONNRESET but it's not correctly taking care of that case, instead it's
incorrectly setting the TCP state to FIN_WAIT_2. It should have set it to
CLOSED and sent a RST to the guest.

Best regards

> 
> 
> If you were to TCP dump the connection in the guest OS you would see
> after killing the "nc" on the host computer that slirp keep acking the
> packets, even though no client application is there.
> 
> 14:55:38.385310 IP 10.0.2.15.4441 > 10.0.2.2.37227: P
> 8884509:8885077(568) ack 2 win 5840
> 14:55:38.385310 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
> 14:55:38.589613 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
> 14:55:38.589613 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
> 14:55:38.997437 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
> 14:55:38.997653 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
> 14:55:39.813522 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
> 14:55:39.813758 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
> 14:55:41.445562 IP 10.0.2.15.4441 > 10.0.2.2.37227: . ack 2 win 5840
> 14:55:41.445769 IP 10.0.2.2.37227 > 10.0.2.15.4441: . ack 8885077 win 0
> 
> 
> The correct behavior should be to send an RST and not an ACK.  There
> might be several ways to correct this problem.  The attached patch is
> one possible way to implement the RFC compliant behavior.  With the
> patch, the tcp dump starts to look like:
> 
> 15:04:34.567350 IP 10.0.2.15.4441 > 10.0.2.2.58510: P
> 2101533:2102993(1460) ack 1 win 5840
> 15:04:34.567350 IP 10.0.2.2.58510 > 10.0.2.15.4441: . ack 2102993 win 5840
> 15:04:34.570718 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2102993
> win 5840
> 15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: .
> 2102993:2104453(1460) ack 1 win 5840
> 15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2104453
> win 4380
> 15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: P
> 2104453:2105345(892) ack 1 win 5840
> 15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: F 1:1(0) ack 2105345
> win 3488
> 15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: . ack 2 win 5840
> 15:04:34.571383 IP 10.0.2.15.4441 > 10.0.2.2.58510: . ack 2 win 5840
> 15:04:34.571383 IP 10.0.2.2.58510 > 10.0.2.15.4441: R
> 12032003:12032003(0) win 3488
> 
> Also with the patch, the SIG_PIPE handlers start to work correctly in
> the guest OS.
> 
> Thanks,
> Jason.

> From: Jason Wessel <jason.wessel@windriver.com>
> Subject: [PATCH] slirp: Fix broken RST response to a slirp redirect socket
> 
> When using slirp networking with a redirected tcp socket, the qemu
> guest os does not receive RST packets when a redirected, accepted
> socket goes into the FIN_WAIT_2 status.  Presently slirp sends ACKs
> instead of RST packets, which means the guest os application socket
> writes do not fail event after the client has terminated the socket.
> 
> This patch changes the behavior to correctly send RST packets instead
> of ACKS.
> 
> Signed-off-by: Jason Wessel <jason.wessel@windriver.com>
> 
> ---
>  slirp/tcp_input.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/slirp/tcp_input.c
> +++ b/slirp/tcp_input.c
> @@ -432,7 +432,7 @@ findso:
>  	tp = sototcpcb(so);
>  
>  	/* XXX Should never fail */
> -	if (tp == 0)
> +	if (tp == 0 || tp->t_state == TCPS_FIN_WAIT_2)
>  		goto dropwithreset;
>  	if (tp->t_state == TCPS_CLOSED)
>  		goto drop;


-- 
Edgar E. Iglesias
Axis Communications AB

Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Edgar E. Iglesias]

On Wed, Jun 11, 2008 at 08:07:39PM +0200, Edgar E. Iglesias wrote:
> On Wed, Jun 11, 2008 at 12:21:45PM -0500, Jason Wessel wrote:
> > 
> > When using slirp networking with a redirected tcp socket, the qemu guest
> > os does not receive RST packets when a redirected, accepted socket goes
> > into the FIN_WAIT_2 status.  Presently slirp sends ACKs instead of RST
> > packets, which means the guest os application socket writes do not fail
> > event after the client has terminated the socket.
> > 
> > Here is a simple way to demonstrate the problem.
> > 
> > * Start qemu with user mode networking plus:
> >      -redir tcp:4441::4441
> > 
> > * Assuming you booted a linux guest os you could run:
> >      cat /dev/zero | nc -p 4441 -l
> > 
> > * On the host run the following command and you
> >   must hit control-c after about 1 second
> >      nc localhost 4441
> 
> Hello Jason,
> 
> IIRC connections in FIN_WAIT_2 can continue to receive data.
> 
> If I might take a wild guess at whats going on:
> The host closed the receiving socket when you ctrl-c nc. That socket still has
> data in it's rcvbuf so the stack aborts the connection and sends a RST. The
> slirp code should now see a -1 on it's next write to that socket and an errno
> ECONNRESET but it's not correctly taking care of that case, instead it's
> incorrectly setting the TCP state to FIN_WAIT_2. It should have set it to
> CLOSED and sent a RST to the guest.

Heh, that guess wasn't entirely correct...
Anyway, here is a patch that hopefully helps.

Best regards
-- 
Edgar E. Iglesias
Axis Communications AB

diff --git a/slirp/socket.c b/slirp/socket.c
index 75003af..2a459a1 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -165,9 +165,21 @@ soread(so)
 		if (nn < 0 && (errno == EINTR || errno == EAGAIN))
 			return 0;
 		else {
+			int err;
+			socklen_t slen;
+
+			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;
 		}
 	}

Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Jason Wessel]

Edgar E. Iglesias wrote:
> On Wed, Jun 11, 2008 at 08:07:39PM +0200, Edgar E. Iglesias wrote:
>> On Wed, Jun 11, 2008 at 12:21:45PM -0500, Jason Wessel wrote:
>>> When using slirp networking with a redirected tcp socket, the qemu guest
>>> os does not receive RST packets when a redirected, accepted socket goes
>>> into the FIN_WAIT_2 status.  Presently slirp sends ACKs instead of RST
>>> packets, which means the guest os application socket writes do not fail
>>> event after the client has terminated the socket.
>>>
>>> Here is a simple way to demonstrate the problem.
>>>
>>> * Start qemu with user mode networking plus:
>>>      -redir tcp:4441::4441
>>>
>>> * Assuming you booted a linux guest os you could run:
>>>      cat /dev/zero | nc -p 4441 -l
>>>
>>> * On the host run the following command and you
>>>   must hit control-c after about 1 second
>>>      nc localhost 4441
>> Hello Jason,
>>
>> IIRC connections in FIN_WAIT_2 can continue to receive data.
>>
>> If I might take a wild guess at whats going on:
>> The host closed the receiving socket when you ctrl-c nc. That socket still has
>> data in it's rcvbuf so the stack aborts the connection and sends a RST. The
>> slirp code should now see a -1 on it's next write to that socket and an errno
>> ECONNRESET but it's not correctly taking care of that case, instead it's
>> incorrectly setting the TCP state to FIN_WAIT_2. It should have set it to
>> CLOSED and sent a RST to the guest.
> 
> Heh, that guess wasn't entirely correct...
> Anyway, here is a patch that hopefully helps.
> 
> Best regards

I'll agree that I didn't look in quite the right place to begin with.

With respect to your patch you might consider making a minor change.



diff --git a/slirp/socket.c b/slirp/socket.c
index 75003af..2a459a1 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -165,9 +165,21 @@ soread(so)
 		if (nn < 0 && (errno == EINTR || errno == EAGAIN))
 			return 0;
 		else {
+			int err;
+			socklen_t slen;
+
+			err = errno;

---

Probably don't need to set err to errno since you are collecting it with getsockopt

---

+			if (nn == 0)
+				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
+					   &err, &slen);

---

In theory you are supposed to set slen = sizeof(err);  prior to calling getsockopt()

The rest looks fine. I used the debugger to step through qemu to double check
it was hitting the right places for the client / server sockets.

---

+
 			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;
 		}
 	}




Jason.

Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Edgar E. Iglesias]

On Wed, Jun 11, 2008 at 03:10:35PM -0500, Jason Wessel wrote:
> +			int err;
> +			socklen_t slen;
> +
> +			err = errno;
> 
> ---
> 
> Probably don't need to set err to errno since you are collecting it with getsockopt

Please note that the errno value is used when nn < 0.

> 
> ---
> 
> +			if (nn == 0)
> +				getsockopt(so->s, SOL_SOCKET, SO_ERROR,
> +					   &err, &slen);
> 
> ---
> 
> In theory you are supposed to set slen = sizeof(err);  prior to calling getsockopt()

Thanks, I'll fix that up.
 
Best regards
-- 
Edgar E. Iglesias
Axis Communications AB

Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket

[Jason Wessel]

Edgar E. Iglesias wrote:
> On Wed, Jun 11, 2008 at 03:10:35PM -0500, Jason Wessel wrote:
>   
>> +			int err;
>> +			socklen_t slen;
>> +
>> +			err = errno;
>>
>> ---
>>
>> Probably don't need to set err to errno since you are collecting it with getsockopt
>>     
>
> Please note that the errno value is used when nn < 0.
>
>   

Oops :-)   I stand corrected, I see how it fits together now.


Cheers,
Jason.

[Qemu-devel] [PATCH] slirp: Propagate host TCP RST to the guest.

[Edgar E. Iglesias]

Hi,

Jason found a bug in teh slirp TCP RST code. Here is a patch to fix it.
Is this OK to commit?

Best regards
-- 
Edgar E. Iglesias
Axis Communications AB

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.

diff --git a/slirp/socket.c b/slirp/socket.c
index 75003af..bb10d69 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -165,9 +165,21 @@ soread(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;
 		}
 	}