slirp: deliver received TCP RSTs to the guest
v2 cover slirp: deliver received TCP RSTs to the guest
Changes from v1:
* added patch 2, a fix for an use-after-free exposed by this series (thanks Samuel Thibault)
* incorporated Reviewed-by lines
* attributed patches correctly
-Steven Luo
===
QEMU's user-mode networking does not currently pass received TCP RSTs to guests, meaning that applications in guests hang if the remote server rejects their network connections. This is particularly noticeable when IPv6 is enabled, the guest is configured to prefer IPv6 and the remote server rejects IPv6 connections (segment-data.zqtk.net is one example), but the bug appears to be longstanding and affects TCP over IPv4 as well.
There are four short patches in this series. The first and second fix crashes which would be exposed by the rest of the series. The third, 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).
Lightly tested on a Linux host with Linux and Windows 7 guests.
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(-)
Signature · 1 line
2.1.4
PATCH v2 1/4 slirp: don't crash when tcp_sockclosed() is called with a NULL tp
From: Steven Luo <[email protected]>
---v1->v2:
* added Reviewed-by line
slirp/tcp_subr.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
slirp/tcp_subr.c +5 -2
@@ -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); }/*
Signature · 1 line
2.1.4
PATCH v2 2/4 slirp: avoid use-after-free in slirp_pollfds_poll() if soread() returns an error
In reply to Steven LuoFrom: Steven Luo <[email protected]>
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.
---
It seems to me that it might be possible to trigger this use-after-free even without the following patches, as tcp_sockclosed() (which soread() currently calls when it gets an error in recv()) frees the socket in certain cases. The remaining patches in this series probably make this much easier to trigger, though.
slirp/slirp.c | 12 +++++++++++- slirp/socket.c | 17 +++++++++++------ slirp/socket.h | 2 +- 3 files changed, 23 insertions(+), 8 deletions(-)
slirp/slirp.c +11 -1
@@ -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 ; + } }/*
slirp/socket.c +11 -6
@@ -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; }/*
slirp/socket.h +1 -1
@@ -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 *);
Signature · 1 line
2.1.4
PATCH v2 3/4 slirp: Propagate host TCP RST to the guest.
In reply to Steven LuoFrom: Edgar E. Iglesias <[email protected]>
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.
---v1->v2:
* fix attribution
slirp/socket.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
slirp/socket.c +16 -1
@@ -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 ; } }
Signature · 1 line
2.1.4
PATCH v2 4/4 slirp: handle deferred ECONNREFUSED on non-blocking TCP sockets
In reply to Steven LuoFrom: Steven Luo <[email protected]>
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.
---v1->v2:
* added Reviewed-by line
slirp/socket.c | 2 +- slirp/tcp_input.c | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-)
slirp/socket.c +1 -1
@@ -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 {
slirp/tcp_input.c +6 -0
@@ -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 ;
Signature · 1 line
2.1.4