[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the gue
From: |
Steven Luo |
Subject: |
Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest |
Date: |
Wed, 6 Apr 2016 17:00:50 -0700 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Apr 06, 2016 at 02:57:43PM +0200, Samuel Thibault wrote:
> address@hidden, on Tue 05 Apr 2016 17:13:58 -0700, wrote:
> > The second,
> > which fixes delivery of an RST interrupting an already-established TCP
> > connection, was submitted by Edgar Iglesias in 2008 and appears to have
> > been missed then. The last patch fixes the case where the remote end
> > sends RST in reply to our SYN (rejects our incoming connection attempt).
>
> It seems I'm getting another crash with these: sowrite would be called
> too for the reseted socket, while the socket has been freed and is not
> even on the polling list any more, I had to additionally do the patch
> below, could you review it so I can push the whole series?
I can't reproduce the crash, but the !(so->so_state & SS_NOFDREF) test
immediately below would seem to be a use-after-free in this case, so I
figure we do need something like this. That said, sorecvoob() also
calls soread(), so I'd guess we need to deal with the possibility that
soread() frees the socket in that case as well? (I can't find any other
callers of soread(), but if they exist, they probably need to be fixed
too.)
I could take care of this when I resend this patch series, if you
prefer.
> It's actually quite easy: just reboot the server :) The new instance of
> the server will send a RST whenever the client sends more data.
Thanks for the hint -- I've verified that case works as well now.
> diff --git a/slirp/slirp.c b/slirp/slirp.c
> index fef526c..b13b9af 100644
> --- a/slirp/slirp.c
> +++ b/slirp/slirp.c
> @@ -553,6 +553,11 @@ void slirp_pollfds_poll(GArray *pollfds, int
> select_error)
> if (ret > 0) {
> tcp_output(sototcpcb(so));
> }
> + if (ret < 0) {
> + /* Socket error and thus removed, do not try to do
> + * anything more with it. */
I think this should be "might have been removed"? tcp_sockclosed()
doesn't seem to call tcp_close() in every case, so we can get -1 from
soread() without the socket being freed.
> + continue;
> + }
> }
>
> /*
-Steven Luo
- [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest, steven, 2016/04/05
- [Qemu-devel] [PATCH 1/3] slirp: don't crash when tcp_sockclosed() is called with a NULL tp, steven, 2016/04/05
- [Qemu-devel] [PATCH 2/3] slirp: Propagate host TCP RST to the guest., steven, 2016/04/05
- [Qemu-devel] [PATCH 3/3] slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets, steven, 2016/04/05
- Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest, Thomas Huth, 2016/04/06
- Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest, Edgar E. Iglesias, 2016/04/06
- Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest, Samuel Thibault, 2016/04/06
- Re: [Qemu-devel] [PATCH 0/3] slirp: deliver received TCP RSTs to the guest,
Steven Luo <=