Fix a crash reported and analyzed by Bertrand PROVOST. When a HTTP

client or server writes multiple requests or chunks in a single
transfer, relayd invokes the libevent callback manually for the
next data.  If the callback closes the session, this resulted in
an use after free.
Instead of the more complicated fix suggested by Bertrand PROVOST,
just move the invocation of the callback to the end of the function.
So in case the callback frees any structures, they are not accessed.
OK benno@ reyk@

1 files changed, 15 insertions, 6 deletions

diff --git a/usr.sbin/relayd/relay_http.c b/usr.sbin/relayd/relay_http.c
index 4e3af605a15..2fdb2d56a7a 100644
--- a/usr.sbin/relayd/relay_http.c
+++ b/usr.sbin/relayd/relay_http.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: relay_http.c,v 1.45 2015/05/18 16:45:16 bluhm Exp $	*/
+/*	$OpenBSD: relay_http.c,v 1.46 2015/05/18 16:57:20 bluhm Exp $	*/
 
 /*
  * Copyright (c) 2006 - 2015 Reyk Floeter <reyk@openbsd.org>
@@ -434,11 +434,18 @@ relay_read_http(struct bufferevent *bev, void *arg)
 		relay_close(con, "last http read (done)");
 		return;
 	}
+	switch (relay_splice(cre)) {
+	case -1:
+		relay_close(con, strerror(errno));
+	case 1:
+		return;
+	case 0:
+		break;
+	}
+	bufferevent_enable(bev, EV_READ);
 	if (EVBUFFER_LENGTH(src) && bev->readcb != relay_read_http)
 		bev->readcb(bev, arg);
-	bufferevent_enable(bev, EV_READ);
-	if (relay_splice(cre) == -1)
-		relay_close(con, strerror(errno));
+	/* The callback readcb() might have freed the session. */
 	return;
  fail:
 	relay_abort_http(con, 500, strerror(errno), 0);
@@ -488,9 +495,10 @@ relay_read_httpcontent(struct bufferevent *bev, void *arg)
 	}
 	if (con->se_done)
 		goto done;
+	bufferevent_enable(bev, EV_READ);
 	if (bev->readcb != relay_read_httpcontent)
 		bev->readcb(bev, arg);
-	bufferevent_enable(bev, EV_READ);
+	/* The callback readcb() might have freed the session. */
 	return;
  done:
 	relay_close(con, "last http content read");
@@ -605,9 +613,10 @@ relay_read_httpchunks(struct bufferevent *bev, void *arg)
  next:
 	if (con->se_done)
 		goto done;
+	bufferevent_enable(bev, EV_READ);
 	if (EVBUFFER_LENGTH(src))
 		bev->readcb(bev, arg);
-	bufferevent_enable(bev, EV_READ);
+	/* The callback readcb() might have freed the session. */
 	return;
 
  done: