From cd66639800ee2882a0867ec54868502eb9b893d8 Mon Sep 17 00:00:00 2001 From: Felix Fietkau Date: Thu, 21 Nov 2013 22:50:30 +0100 Subject: [PATCH] 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 --- client.c | 7 ++++++- ubus.c | 8 ++++++++ uhttpd.h | 16 ++++++++++++++++ utils.c | 9 +++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) 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"); } -- 2.25.1