uhttpd: fix crashes in the ubus plugin

The ubus plugin calls blocking ubus functions that loop back into
uloop_run. Protect the client data structure with refcounting to ensure
that the outer uloop_run call does not clean up the data that the inner
uloop_run call is still processing.

Signed-off-by: Felix Fietkau <nbd@openwrt.org>

4 files changed, 39 insertions, 1 deletions

diff --git a/client.c b/client.c
index 3185b8f..3a6b09e 100644
--- a/client.c
+++ b/client.c
@@ -463,6 +463,11 @@ void uh_client_read_cb(struct client *cl)
 
 static void client_close(struct client *cl)
 {
+	if (cl->refcount) {
+		cl->state = CLIENT_STATE_CLEANUP;
+		return;
+	}
+
 	client_done = true;
 	n_clients--;
 	uh_dispatch_done(cl);
@@ -482,7 +487,7 @@ void uh_client_notify_state(struct client *cl)
 {
 	struct ustream *s = cl->us;
 
-	if (!s->write_error) {
+	if (!s->write_error && cl->state != CLIENT_STATE_CLEANUP) {
 		if (cl->state == CLIENT_STATE_DATA)
 			return;
 
diff --git a/ubus.c b/ubus.c
index cf5f529..da1ab60 100644
--- a/ubus.c
+++ b/ubus.c
@@ -332,6 +332,8 @@ static void uh_ubus_send_list(struct client *cl, json_object *obj, struct blob_a
 
 	blob_buf_init(data.buf, 0);
 
+	uh_client_ref(cl);
+
 	if (!params || blob_id(params) != BLOBMSG_TYPE_ARRAY) {
 		r = blobmsg_open_array(data.buf, "result");
 		ubus_lookup(ctx, NULL, uh_ubus_list_cb, &data);
@@ -351,6 +353,8 @@ static void uh_ubus_send_list(struct client *cl, json_object *obj, struct blob_a
 		blobmsg_close_table(data.buf, r);
 	}
 
+	uh_client_unref(cl);
+
 	uh_ubus_init_response(cl);
 	blobmsg_add_blob(&buf, blob_data(data.buf->head));
 	uh_ubus_send_response(cl);
@@ -463,6 +467,8 @@ static void uh_ubus_handle_request_object(struct client *cl, struct json_object
 	struct rpc_data data = {};
 	enum rpc_error err = ERROR_PARSE;
 
+	uh_client_ref(cl);
+
 	if (json_object_get_type(obj) != json_type_object)
 		goto error;
 
@@ -506,6 +512,8 @@ error:
 out:
 	if (data.params)
 		free(data.params);
+
+	uh_client_unref(cl);
 }
 
 static void __uh_ubus_next_batched_request(struct uloop_timeout *timeout)
diff --git a/uhttpd.h b/uhttpd.h
index b289a24..a620030 100644
--- a/uhttpd.h
+++ b/uhttpd.h
@@ -118,6 +118,7 @@ enum client_state {
 	CLIENT_STATE_DATA,
 	CLIENT_STATE_DONE,
 	CLIENT_STATE_CLOSE,
+	CLIENT_STATE_CLEANUP,
 };
 
 struct interpreter {
@@ -223,6 +224,7 @@ struct dispatch {
 
 struct client {
 	struct list_head list;
+	int refcount;
 	int id;
 
 	struct ustream *us;
@@ -298,4 +300,18 @@ bool uh_create_process(struct client *cl, struct path_info *pi, char *url,
 int uh_plugin_init(const char *name);
 void uh_plugin_post_init(void);
 
+static inline void uh_client_ref(struct client *cl)
+{
+	cl->refcount++;
+}
+
+static inline void uh_client_unref(struct client *cl)
+{
+	if (--cl->refcount)
+		return;
+
+	if (cl->state == CLIENT_STATE_CLEANUP)
+		ustream_state_change(cl->us);
+}
+
 #endif
diff --git a/utils.c b/utils.c
index 3b868e3..b1d7d37 100644
--- a/utils.c
+++ b/utils.c
@@ -35,6 +35,9 @@ void uh_chunk_write(struct client *cl, const void *data, int len)
 {
 	bool chunked = uh_use_chunked(cl);
 
+	if (cl->state == CLIENT_STATE_CLEANUP)
+		return;
+
 	uloop_timeout_set(&cl->timeout, conf.network_timeout * 1000);
 	if (chunked)
 		ustream_printf(cl->us, "%X\r\n", len);
@@ -49,6 +52,9 @@ void uh_chunk_vprintf(struct client *cl, const char *format, va_list arg)
 	va_list arg2;
 	int len;
 
+	if (cl->state == CLIENT_STATE_CLEANUP)
+		return;
+
 	uloop_timeout_set(&cl->timeout, conf.network_timeout * 1000);
 	if (!uh_use_chunked(cl)) {
 		ustream_vprintf(cl->us, format, arg);
@@ -81,6 +87,9 @@ void uh_chunk_eof(struct client *cl)
 	if (!uh_use_chunked(cl))
 		return;
 
+	if (cl->state == CLIENT_STATE_CLEANUP)
+		return;
+
 	ustream_printf(cl->us, "0\r\n\r\n");
 }