Skip to contentSkip to conversation

Proposed fix broken RST response to a slirp redirect socket

PATCH In review Jason Wessel 2 participants 7 messages Jun 11, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Proposed 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.

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.

From: Jason Wessel <[email protected]>
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.

---
 slirp/tcp_input.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
slirp/tcp_input.c +1 -1
@@ -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;
#1 lore raw
Quoted 10 lines · Jason Wessel

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:

Quoted 8 lines
     -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

Quoted 77 lines

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 <[email protected]>
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.

---
 slirp/tcp_input.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
slirp/tcp_input.c +1 -1
@@ -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;
Signature · 2 lines
Edgar E. Iglesias
Axis Communications AB
#2 lore raw
Quoted 24 lines · Edgar E. Iglesias
Quoted 10 lines · Jason Wessel

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:

Quoted 8 lines
     -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.

Quoted 7 lines

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

Signature · 2 lines
Edgar E. Iglesias
Axis Communications AB
slirp/socket.c +13 -1
index 75003af..2a459a1 100644
@@ -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;
 		}
 	}
#3 lore raw
Quoted 31 lines · Edgar E. Iglesias
Quoted 22 lines · Edgar E. Iglesias
Quoted 9 lines · Jason Wessel

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:

Quoted 8 lines
     -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.

Quoted 7 lines

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.

Quoted 4 lines

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.

slirp/socket.c +4 -1
index 75003af..2a459a1 100644
@@ -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.

#4 lore raw
Quoted 8 lines · Jason Wessel
slirp/socket.c +4 -1
+			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.

Quoted 10 lines

---

(quoted diff) +3 -1
+			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

Signature · 2 lines
Edgar E. Iglesias
Axis Communications AB
#5 lore raw
Quoted 7 lines · Edgar E. Iglesias
Quoted 5 lines · Jason Wessel
slirp/socket.c +4 -0
+			int err;
+			socklen_t slen;
+
+			err = errno;
Quoted 8 lines
Quoted 4 lines

---

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.

#6 lore raw

PATCH 1/1 slirp: Propagate host TCP RST to the guest.

In reply to Jason Wessel

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

Signature · 6 lines
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.
slirp/socket.c +13 -1
index 75003af..bb10d69 100644
@@ -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;
 		}
 	}