[Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Samuel Thibault]

The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:

  Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to b9b314378ce0fdb4996a0316cb34246f746ac8cf:

  slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 10:28:53 +0200)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Edgar E. Iglesias (1):
      slirp: Propagate host TCP RST to the guest.

Steven Luo (3):
      slirp: don't crash when tcp_sockclosed() is called with a NULL tp
      slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
      slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

 slirp/slirp.c     | 12 +++++++++++-
 slirp/socket.c    | 34 +++++++++++++++++++++++++++-------
 slirp/socket.h    |  2 +-
 slirp/tcp_input.c |  6 ++++++
 slirp/tcp_subr.c  |  7 +++++--
 5 files changed, 50 insertions(+), 11 deletions(-)

[Qemu-devel] [PULL 1/4] slirp: don't crash when tcp_sockclosed() is called with a NULL tp

[Samuel Thibault]

From: Steven Luo <steven+qemu@steven676.net>

Signed-off-by: Steven Luo <steven+qemu@steven676.net>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/tcp_subr.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/slirp/tcp_subr.c b/slirp/tcp_subr.c
index dbfd2c6..32ff452 100644
--- a/slirp/tcp_subr.c
+++ b/slirp/tcp_subr.c
@@ -356,6 +356,10 @@ tcp_sockclosed(struct tcpcb *tp)
 	DEBUG_CALL("tcp_sockclosed");
 	DEBUG_ARG("tp = %p", tp);
 
+	if (!tp) {
+		return;
+	}
+
 	switch (tp->t_state) {
 
 	case TCPS_CLOSED:
@@ -374,8 +378,7 @@ tcp_sockclosed(struct tcpcb *tp)
 		tp->t_state = TCPS_LAST_ACK;
 		break;
 	}
-	if (tp)
-		tcp_output(tp);
+	tcp_output(tp);
 }
 
 /*
-- 
2.8.0.rc3

[Qemu-devel] [PULL 2/4] slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error

[Samuel Thibault]

From: Steven Luo <steven+qemu@steven676.net>

Samuel Thibault pointed out that it's possible that slirp_pollfds_poll()
will try to use a socket even after soread() returns an error, resulting
in an use-after-free if the socket was removed while handling the error.
Avoid this by refusing to continue to work with the socket in this case.

Signed-off-by: Steven Luo <steven+qemu@steven676.net>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/slirp.c  | 12 +++++++++++-
 slirp/socket.c | 17 +++++++++++------
 slirp/socket.h |  2 +-
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/slirp/slirp.c b/slirp/slirp.c
index fef526c..9f4bea3 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -534,7 +534,12 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
                  * test for G_IO_IN below if this succeeds
                  */
                 if (revents & G_IO_PRI) {
-                    sorecvoob(so);
+                    ret = sorecvoob(so);
+                    if (ret < 0) {
+                        /* Socket error might have resulted in the socket being
+                         * removed, do not try to do anything more with it. */
+                        continue;
+                    }
                 }
                 /*
                  * Check sockets for reading
@@ -553,6 +558,11 @@ void slirp_pollfds_poll(GArray *pollfds, int select_error)
                     if (ret > 0) {
                         tcp_output(sototcpcb(so));
                     }
+                    if (ret < 0) {
+                        /* Socket error might have resulted in the socket being
+                         * removed, do not try to do anything more with it. */
+                        continue;
+                    }
                 }
 
                 /*
diff --git a/slirp/socket.c b/slirp/socket.c
index b836c42..7f022a6 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -260,10 +260,11 @@ err:
  * so when OOB data arrives, we soread() it and everything
  * in the send buffer is sent as urgent data
  */
-void
+int
 sorecvoob(struct socket *so)
 {
 	struct tcpcb *tp = sototcpcb(so);
+	int ret;
 
 	DEBUG_CALL("sorecvoob");
 	DEBUG_ARG("so = %p", so);
@@ -276,11 +277,15 @@ sorecvoob(struct socket *so)
 	 * urgent data, or the read() doesn't return all the
 	 * urgent data.
 	 */
-	soread(so);
-	tp->snd_up = tp->snd_una + so->so_snd.sb_cc;
-	tp->t_force = 1;
-	tcp_output(tp);
-	tp->t_force = 0;
+	ret = soread(so);
+	if (ret > 0) {
+	    tp->snd_up = tp->snd_una + so->so_snd.sb_cc;
+	    tp->t_force = 1;
+	    tcp_output(tp);
+	    tp->t_force = 0;
+	}
+
+	return ret;
 }
 
 /*
diff --git a/slirp/socket.h b/slirp/socket.h
index e9c9b05..7dca506 100644
--- a/slirp/socket.h
+++ b/slirp/socket.h
@@ -127,7 +127,7 @@ struct socket *solookup(struct socket **, struct socket *,
 struct socket *socreate(Slirp *);
 void sofree(struct socket *);
 int soread(struct socket *);
-void sorecvoob(struct socket *);
+int sorecvoob(struct socket *);
 int sosendoob(struct socket *);
 int sowrite(struct socket *);
 void sorecvfrom(struct socket *);
-- 
2.8.0.rc3

[Qemu-devel] [PULL 3/4] slirp: Propagate host TCP RST to the guest.

[Samuel Thibault]

From: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>

When the host aborts (RST) its 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>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/socket.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 7f022a6..0d67b12 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.8.0.rc3

[Qemu-devel] [PULL 4/4] slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

[Samuel Thibault]

From: Steven Luo <steven+qemu@steven676.net>

slirp currently only handles ECONNREFUSED in the case where connect()
returns immediately with that error; since we use non-blocking sockets,
most of the time we won't receive the error until we later try to read
from the socket.  Ensure that we deliver the appropriate RST to the
guest in this case.

Signed-off-by: Steven Luo <steven+qemu@steven676.net>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
---
 slirp/socket.c    | 2 +-
 slirp/tcp_input.c | 6 ++++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/slirp/socket.c b/slirp/socket.c
index 0d67b12..bd97b2d 100644
--- a/slirp/socket.c
+++ b/slirp/socket.c
@@ -188,7 +188,7 @@ soread(struct socket *so)
 			DEBUG_MISC((dfd, " --- soread() disconnected, nn = %d, errno = %d-%s\n", nn, errno,strerror(errno)));
 			sofcantrcvmore(so);
 
-			if (err == ECONNRESET
+			if (err == ECONNRESET || err == ECONNREFUSED
 			    || err == ENOTCONN || err == EPIPE) {
 				tcp_drop(sototcpcb(so), err);
 			} else {
diff --git a/slirp/tcp_input.c b/slirp/tcp_input.c
index 1fcca30..5433e7f 100644
--- a/slirp/tcp_input.c
+++ b/slirp/tcp_input.c
@@ -725,6 +725,12 @@ findso:
 	    so->so_ti = ti;
 	    tp->t_timer[TCPT_KEEP] = TCPTV_KEEP_INIT;
 	    tp->t_state = TCPS_SYN_RECEIVED;
+	    /*
+	     * Initialize receive sequence numbers now so that we can send a
+	     * valid RST if the remote end rejects our connection.
+	     */
+	    tp->irs = ti->ti_seq;
+	    tcp_rcvseqinit(tp);
 	    tcp_template(tp);
 	  }
 	  return;
-- 
2.8.0.rc3

Re: [Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Peter Maydell]

On 7 April 2016 at 09:34, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:
>
>   Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to b9b314378ce0fdb4996a0316cb34246f746ac8cf:
>
>   slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 10:28:53 +0200)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------
> Edgar E. Iglesias (1):
>       slirp: Propagate host TCP RST to the guest.
>
> Steven Luo (3):
>       slirp: don't crash when tcp_sockclosed() is called with a NULL tp
>       slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
>       slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

The patch from Edgar appears to be missing Edgar's signed-off-by ?

thanks
-- PMM

Re: [Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Edgar E. Iglesias]

On Thu, Apr 07, 2016 at 11:13:37AM +0100, Peter Maydell wrote:
> On 7 April 2016 at 09:34, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> > The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:
> >
> >   Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)
> >
> > are available in the git repository at:
> >
> >   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
> >
> > for you to fetch changes up to b9b314378ce0fdb4996a0316cb34246f746ac8cf:
> >
> >   slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 10:28:53 +0200)
> >
> > ----------------------------------------------------------------
> > slirp updates
> >
> > ----------------------------------------------------------------
> > Edgar E. Iglesias (1):
> >       slirp: Propagate host TCP RST to the guest.
> >
> > Steven Luo (3):
> >       slirp: don't crash when tcp_sockclosed() is called with a NULL tp
> >       slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
> >       slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets
> 
> The patch from Edgar appears to be missing Edgar's signed-off-by ?

Yes, this patch is ancient from times when SoBs were not commonly used but feel free to add one:

Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Thanks,
Edgar


> 
> thanks
> -- PMM

Re: [Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Peter Maydell]

On 7 April 2016 at 11:27, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> On Thu, Apr 07, 2016 at 11:13:37AM +0100, Peter Maydell wrote:
>> On 7 April 2016 at 09:34, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
>> > The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:
>> >
>> >   Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)
>> >
>> > are available in the git repository at:
>> >
>> >   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>> >
>> > for you to fetch changes up to b9b314378ce0fdb4996a0316cb34246f746ac8cf:
>> >
>> >   slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 10:28:53 +0200)
>> >
>> > ----------------------------------------------------------------
>> > slirp updates
>> >
>> > ----------------------------------------------------------------
>> > Edgar E. Iglesias (1):
>> >       slirp: Propagate host TCP RST to the guest.
>> >
>> > Steven Luo (3):
>> >       slirp: don't crash when tcp_sockclosed() is called with a NULL tp
>> >       slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
>> >       slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets
>>
>> The patch from Edgar appears to be missing Edgar's signed-off-by ?
>
> Yes, this patch is ancient from times when SoBs were not commonly used but feel free to add one:
>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>

Thanks; Samuel, can I ask you to respin with that line added?

-- PMM

[Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Samuel Thibault]

The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:

  Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)

are available in the git repository at:

  http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault

for you to fetch changes up to 6625d83a6eb3b51a622d72adce713cab75cbf2e7:

  slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 13:02:05 +0200)

----------------------------------------------------------------
slirp updates

----------------------------------------------------------------
Edgar E. Iglesias (1):
      slirp: Propagate host TCP RST to the guest.

Steven Luo (3):
      slirp: don't crash when tcp_sockclosed() is called with a NULL tp
      slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
      slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets

 slirp/slirp.c     | 12 +++++++++++-
 slirp/socket.c    | 34 +++++++++++++++++++++++++++-------
 slirp/socket.h    |  2 +-
 slirp/tcp_input.c |  6 ++++++
 slirp/tcp_subr.c  |  7 +++++--
 5 files changed, 50 insertions(+), 11 deletions(-)

Re: [Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Samuel Thibault]

Peter Maydell, on Thu 07 Apr 2016 11:37:45 +0100, wrote:
> Thanks; Samuel, can I ask you to respin with that line added?

Done so.

Samuel

Re: [Qemu-devel] [PULL 0/4] slirp: deliver received TCP RSTs to the guest

[Peter Maydell]

On 7 April 2016 at 12:04, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> The following changes since commit 7acbff99c6c285b3070bf0e768d56f511e2bf346:
>
>   Update version for v2.6.0-rc1 release (2016-04-05 21:53:18 +0100)
>
> are available in the git repository at:
>
>   http://people.debian.org/~sthibault/qemu.git tags/samuel-thibault
>
> for you to fetch changes up to 6625d83a6eb3b51a622d72adce713cab75cbf2e7:
>
>   slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets (2016-04-07 13:02:05 +0200)
>
> ----------------------------------------------------------------
> slirp updates
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM