From b5ab677189b93efa4eaa921f42b21dc008247184 Mon Sep 17 00:00:00 2001 From: Steven Luo Date: Wed, 6 Apr 2016 22:04:21 -0700 Subject: [PATCH 1/4] slirp: don't crash when tcp_sockclosed() is called with a NULL tp Signed-off-by: Steven Luo Reviewed-by: Edgar E. Iglesias Signed-off-by: Samuel Thibault --- 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 dbfd2c673b..32ff452e93 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); } /* From bfb1ac14029ee72b19296109fba880c0551755d5 Mon Sep 17 00:00:00 2001 From: Steven Luo Date: Wed, 6 Apr 2016 22:04:32 -0700 Subject: [PATCH 2/4] slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error 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 Signed-off-by: Samuel Thibault --- 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 fef526c5ad..9f4bea3d3b 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 b836c42b8e..7f022a6769 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 e9c9b053dc..7dca506973 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 *); From 27d92ebc5ed1bb0b518d0ebc4c609182ad20a799 Mon Sep 17 00:00:00 2001 From: "Edgar E. Iglesias" Date: Wed, 6 Apr 2016 22:04:43 -0700 Subject: [PATCH 3/4] slirp: Propagate host TCP RST to the guest. 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. Signed-off-by: Edgar E. Iglesias [steven@steven676.net: coding style adjustments] Signed-off-by: Steven Luo Signed-off-by: Samuel Thibault --- slirp/socket.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/slirp/socket.c b/slirp/socket.c index 7f022a6769..0d67b12678 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; } } From 6625d83a6eb3b51a622d72adce713cab75cbf2e7 Mon Sep 17 00:00:00 2001 From: Steven Luo Date: Wed, 6 Apr 2016 22:04:55 -0700 Subject: [PATCH 4/4] slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets 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 Reviewed-by: Edgar E. Iglesias Signed-off-by: Samuel Thibault --- 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 0d67b12678..bd97b2d682 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 1fcca3040e..5433e7fe9c 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;