commit 390d312b22670d92dc6ee5afd7a116b7a2330881
from: Omar Polo <op@omarpolo.com>
date: Wed Aug 09 19:13:13 2023 UTC

don't call client_close() from fcgi/proxy bev handlers

We might end up calling client_close() from start_reply(), but that
will free the fcgi/proxy bufferevent while they're still used on the
stack.

Instead, start_reply() only sets REQUEST_DONE and exits, returning the
error eventually, so callers know when to stop.

commit - 01481c255ae837d80f00ffcf8493e5b13b329323
commit + 390d312b22670d92dc6ee5afd7a116b7a2330881
blob - 69613d24fcfcdbc8e59997a425351f2d854ae0e5
blob + b8378d40a373badf9fe3fa8dd24c6dd7751f1799
--- fcgi.c
+++ fcgi.c
@@ -258,8 +258,8 @@ fcgi_handle_stdout(struct client *c, struct evbuffer *
 			return;
 		}
 
-		start_reply(c, code, c->sbuf + 3);
-		if (c->code < 20 || c->code > 29) {
+		if (start_reply(c, code, c->sbuf + 3) == -1 ||
+		    c->code < 20 || c->code > 29) {
 			fcgi_error(bev, EVBUFFER_EOF, c);
 			return;
 		}
@@ -280,7 +280,7 @@ fcgi_read(struct bufferevent *bev, void *d)
 	struct fcgi_end_req_body end;
 	size_t			 len;
 
-	for (;;) {
+	while (c->type != REQUEST_DONE) {
 		if (EVBUFFER_LENGTH(src) < sizeof(hdr))
 			return;
 
@@ -310,8 +310,7 @@ fcgi_read(struct bufferevent *bev, void *d)
 
 			/* TODO: do something with the status? */
 			c->type = REQUEST_DONE;
-			client_write(c->bev, c);
-			return;
+			break;
 
 		case FCGI_STDERR:
 			/* discard stderr (for now) */
@@ -333,6 +332,7 @@ fcgi_read(struct bufferevent *bev, void *d)
 
 err:
 	fcgi_error(bev, EVBUFFER_ERROR, c);
+	client_write(c->bev, c);
 }
 
 void
@@ -352,10 +352,12 @@ fcgi_error(struct bufferevent *bev, short err, void *d
 	/*
 	 * If we're here it means that some kind of non-recoverable
 	 * error happened.
+	 *
+	 * Don't free bev as we might be called by a function that
+	 * still uses it.
 	 */
 
-	bufferevent_free(bev);
-	c->cgibev = NULL;
+	bufferevent_disable(bev, EVBUFFER_READ);
 
 	close(c->pfd);
 	c->pfd = -1;
@@ -367,7 +369,6 @@ fcgi_error(struct bufferevent *bev, short err, void *d
 	}
 
 	c->type = REQUEST_DONE;
-	client_write(c->bev, c);
 }
 
 void
blob - bdf38839422a91150fc3ded6145ef4ffc1de3afd
blob + 7651f9bc188dd361ea875e8fd5a4c8683dd84f49
--- gmid.h
+++ gmid.h
@@ -409,7 +409,7 @@ int		 vhost_disable_log(struct vhost*, const char*);
 
 void		 mark_nonblock(int);
 void		 client_write(struct bufferevent *, void *);
-void		 start_reply(struct client*, int, const char*);
+int		 start_reply(struct client*, int, const char*);
 void		 client_close(struct client *);
 struct client	*client_by_id(int);
 void		 server_accept(int, short, void *);
blob - aa2c35783a48a2d6b1489555131641fb21d90fc1
blob + a3ccbef66ab1c01d7f5c6cc76d709f3022792e97
--- proxy.c
+++ proxy.c
@@ -182,9 +182,8 @@ proxy_read(struct bufferevent *bev, void *d)
 			return;
 		}
 
-		start_reply(c, code, hdr + 3);
-
-		if (c->code < 20 || c->code > 29) {
+		if (start_reply(c, code, hdr + 3) == -1 ||
+		    c->code < 20 || c->code > 29) {
 			proxy_error(bev, EVBUFFER_EOF, c);
 			return;
 		}
blob - 4678e3fd586300578164b8eaf40b9d8192dcb6dc
blob + 4e163b0837e1a091e1522f6b6d9b5626ca493005
--- server.c
+++ server.c
@@ -1122,7 +1122,7 @@ client_error(struct bufferevent *bev, short error, voi
 	client_close(c);
 }
 
-void
+int
 start_reply(struct client *c, int code, const char *meta)
 {
 	struct evbuffer	*evb = EVBUFFER_OUTPUT(c->bev);
@@ -1161,18 +1161,19 @@ start_reply(struct client *c, int code, const char *me
 	if (code != 20)
 		c->type = REQUEST_DONE;
 
-	return;
+	return 0;
 
 err:
 	log_warnx("evbuffer_add_printf error: no memory");
 	evbuffer_drain(evb, EVBUFFER_LENGTH(evb));
-	client_close(c);
-	return;
+	c->type = REQUEST_DONE;
+	return -1;
 
 overflow:
 	log_warnx("reply header overflow");
 	evbuffer_drain(evb, EVBUFFER_LENGTH(evb));
 	start_reply(c, TEMP_FAILURE, "internal error");
+	return -1;
 }
 
 static void