qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Jason Wessel
Subject: Re: [Qemu-devel] [PATCH] Proposed fix broken RST response to a slirp redirect socket
Date: Wed, 11 Jun 2008 15:10:35 -0500
User-agent: Thunderbird 2.0.0.14 (X11/20080502)

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.




reply via email to

[Prev in Thread] Current Thread [Next in Thread]