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>

diff --git a/client.c b/client.c
index 3185b8fd1a14d7b4d94ee1c0178c60de8090a0d4..3a6b09eea20b36b23cf59c05714c55f85fc18b6f 100644 (file)
--- 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 cf5f529d3496d7942eccbe40aaf9ba72bcf1e417..da1ab606762871222768ac768812660cb54bf831 100644 (file)
--- 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 b289a24edc817b52934c5c49d69c26b512c46c3c..a620030ae20962684a360d1f2c8cd6664f451c00 100644 (file)
--- 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 3b868e3576805126a05283c9ce38ee60c407988c..b1d7d37c68cd311525b35d650133590a6493f464 100644 (file)
--- 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");
 }