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

Commit Message

Steven Luo April 7, 2016, 5:04 a.m. UTC

From: Steven Luo <[e-mail]>

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 <[e-mail]>
---
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(-)

Patch

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 *);