From e24c934a1b10d70d6bbecfd9317d0f5e03ca10c2 Mon Sep 17 00:00:00 2001 From: Wim Taymans Date: Tue, 1 Aug 2017 20:11:38 +0200 Subject: [PATCH] protocol-native: improve demarshal Add flags to demarshal functions and check remap and write access in just one place. --- src/extensions/protocol-native.h | 8 ++ .../module-client-node/protocol-native.c | 52 +++++------- src/modules/module-protocol-native.c | 79 +++++++++++++------ .../module-protocol-native/protocol-native.c | 64 ++++++--------- 4 files changed, 109 insertions(+), 94 deletions(-) diff --git a/src/extensions/protocol-native.h b/src/extensions/protocol-native.h index 232d7db26..3a174bc7f 100644 --- a/src/extensions/protocol-native.h +++ b/src/extensions/protocol-native.h @@ -33,6 +33,14 @@ extern "C" { #define PW_TYPE_PROTOCOL__Native PW_TYPE_PROTOCOL_BASE "Native" #define PW_TYPE_PROTOCOL_NATIVE_BASE PW_TYPE_PROTOCOL__Native ":" +struct pw_protocol_native_demarshal { + bool (*func) (void *object, void *data, size_t size); + +#define PW_PROTOCOL_NATIVE_REMAP (1<<0) +#define PW_PROTOCOL_NATIVE_PERM_W (1<<1) + uint32_t flags; +}; + /** \ref pw_protocol_native_ext methods */ struct pw_protocol_native_ext { #define PW_VERSION_PROTOCOL_NATIVE_EXT 0 diff --git a/src/modules/module-client-node/protocol-native.c b/src/modules/module-client-node/protocol-native.c index d6eb28b20..d5589b637 100644 --- a/src/modules/module-client-node/protocol-native.c +++ b/src/modules/module-client-node/protocol-native.c @@ -29,12 +29,6 @@ #include "extensions/protocol-native.h" #include "extensions/client-node.h" -/** \cond */ - -typedef bool(*demarshal_func_t) (void *object, void *data, size_t size); - -/** \endcond */ - static void client_node_marshal_done(void *object, int seq, int res) { @@ -169,7 +163,6 @@ static bool client_node_demarshal_event_event(void *object, void *data, size_t s const struct spa_event *event; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_OBJECT, &event, 0)) return false; @@ -217,7 +210,6 @@ static bool client_node_demarshal_set_format(void *object, void *data, size_t si const struct spa_format *format = NULL; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &seq, SPA_POD_TYPE_INT, &direction, @@ -239,7 +231,6 @@ static bool client_node_demarshal_set_param(void *object, void *data, size_t siz const struct spa_param *param = NULL; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &seq, SPA_POD_TYPE_INT, &direction, @@ -259,7 +250,6 @@ static bool client_node_demarshal_add_mem(void *object, void *data, size_t size) int memfd; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &direction, SPA_POD_TYPE_INT, &port_id, @@ -289,7 +279,6 @@ static bool client_node_demarshal_use_buffers(void *object, void *data, size_t s int i, j; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &seq, SPA_POD_TYPE_INT, &direction, @@ -350,7 +339,6 @@ static bool client_node_demarshal_node_command(void *object, void *data, size_t uint32_t seq; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &seq, SPA_POD_TYPE_OBJECT, &command, 0)) return false; @@ -366,7 +354,6 @@ static bool client_node_demarshal_port_command(void *object, void *data, size_t uint32_t direction, port_id; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &direction, SPA_POD_TYPE_INT, &port_id, @@ -671,7 +658,6 @@ static bool client_node_demarshal_update(void *object, void *data, size_t size) const struct spa_props *props; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &change_mask, SPA_POD_TYPE_INT, &max_input_ports, @@ -696,7 +682,6 @@ static bool client_node_demarshal_port_update(void *object, void *data, size_t s struct spa_pod *ipod; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &direction, SPA_POD_TYPE_INT, &port_id, @@ -749,7 +734,6 @@ static bool client_node_demarshal_event_method(void *object, void *data, size_t struct spa_event *event; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_OBJECT, &event, 0)) return false; @@ -778,12 +762,12 @@ static const struct pw_client_node_methods pw_protocol_native_client_node_method &client_node_marshal_destroy }; -static const demarshal_func_t pw_protocol_native_client_node_method_demarshal[] = { - &client_node_demarshal_done, - &client_node_demarshal_update, - &client_node_demarshal_port_update, - &client_node_demarshal_event_method, - &client_node_demarshal_destroy, +static const struct pw_protocol_native_demarshal pw_protocol_native_client_node_method_demarshal[] = { + { &client_node_demarshal_done, 0 }, + { &client_node_demarshal_update, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_port_update, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_event_method, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_destroy, 0 }, }; static const struct pw_client_node_events pw_protocol_native_client_node_event_marshal = { @@ -801,18 +785,18 @@ static const struct pw_client_node_events pw_protocol_native_client_node_event_m &client_node_marshal_port_command, }; -static const demarshal_func_t pw_protocol_native_client_node_event_demarshal[] = { - &client_node_demarshal_transport, - &client_node_demarshal_set_props, - &client_node_demarshal_event_event, - &client_node_demarshal_add_port, - &client_node_demarshal_remove_port, - &client_node_demarshal_set_format, - &client_node_demarshal_set_param, - &client_node_demarshal_add_mem, - &client_node_demarshal_use_buffers, - &client_node_demarshal_node_command, - &client_node_demarshal_port_command, +static const struct pw_protocol_native_demarshal pw_protocol_native_client_node_event_demarshal[] = { + { &client_node_demarshal_transport, 0 }, + { &client_node_demarshal_set_props, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_event_event, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_add_port, 0 }, + { &client_node_demarshal_remove_port, 0 }, + { &client_node_demarshal_set_format, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_set_param, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_add_mem, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_use_buffers, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_node_command, PW_PROTOCOL_NATIVE_REMAP }, + { &client_node_demarshal_port_command, PW_PROTOCOL_NATIVE_REMAP }, }; const struct pw_protocol_marshal pw_protocol_native_client_node_marshal = { diff --git a/src/modules/module-protocol-native.c b/src/modules/module-protocol-native.c index e7c8c6cab..f0f761566 100644 --- a/src/modules/module-protocol-native.c +++ b/src/modules/module-protocol-native.c @@ -54,8 +54,6 @@ void pw_protocol_native_init(struct pw_protocol *protocol); -typedef bool(*demarshal_func_t) (void *object, void *data, size_t size); - struct protocol_data { struct pw_module *module; }; @@ -106,7 +104,7 @@ process_messages(struct pw_client *client) while (pw_protocol_native_connection_get_next(conn, &opcode, &id, &message, &size)) { struct pw_resource *resource; - const demarshal_func_t *demarshal; + const struct pw_protocol_native_demarshal *demarshal; pw_log_trace("protocol-native %p: got message %d from %u", client->protocol, opcode, id); @@ -117,23 +115,50 @@ process_messages(struct pw_client *client) client->protocol, id); continue; } - if (opcode >= resource->marshal->n_methods) { - pw_log_error("protocol-native %p: invalid method %u %u", client->protocol, - id, opcode); - pw_client_destroy(client); - break; + if ((resource->permissions & PW_PERM_X) == 0) { + pw_log_error("protocol-native %p: execute not allowed on resource %u", + client->protocol, id); + continue; } + + if (opcode >= resource->marshal->n_methods) + goto invalid_method; + demarshal = resource->marshal->method_demarshal; - if (!demarshal[opcode] || !demarshal[opcode] (resource, message, size)) { - pw_log_error("protocol-native %p: invalid message received %u %u", - client->protocol, id, opcode); - pw_client_destroy(client); - break; + if (!demarshal[opcode].func) + goto invalid_message; + + if ((demarshal[opcode].flags & PW_PROTOCOL_NATIVE_PERM_W) && + ((resource->permissions & PW_PERM_X) == 0)) { + pw_log_error("protocol-native %p: method %u requires write access on %u", + client->protocol, opcode, id); + continue; } - if (client->busy) { + + if (demarshal[opcode].flags & PW_PROTOCOL_NATIVE_REMAP) + if (!pw_pod_remap_data(SPA_POD_TYPE_STRUCT, message, size, &client->types)) + goto invalid_message; + + if (!demarshal[opcode].func (resource, message, size)) + goto invalid_message; + + /* when the client is busy processing an async action, stop processing messages + * for the client until it finishes the action */ + if (client->busy) break; - } } + return; + + invalid_method: + pw_log_error("protocol-native %p: invalid method %u on resource %u", + client->protocol, opcode, id); + pw_client_destroy(client); + return; + invalid_message: + pw_log_error("protocol-native %p: invalid message received %u %u", + client->protocol, id, opcode); + pw_client_destroy(client); + return; } static void @@ -444,7 +469,7 @@ on_remote_data(struct spa_loop_utils *utils, while (!impl->disconnecting && pw_protocol_native_connection_get_next(conn, &opcode, &id, &message, &size)) { struct pw_proxy *proxy; - const demarshal_func_t *demarshal; + const struct pw_protocol_native_demarshal *demarshal; pw_log_trace("protocol-native %p: got message %d from %u", this, opcode, id); @@ -460,15 +485,25 @@ on_remote_data(struct spa_loop_utils *utils, } demarshal = proxy->marshal->event_demarshal; - if (demarshal[opcode]) { - if (!demarshal[opcode] (proxy, message, size)) + if (!demarshal[opcode].func) { + pw_log_error("protocol-native %p: function %d not implemented on %u", this, + opcode, id); + continue; + } + + if (demarshal[opcode].flags & PW_PROTOCOL_NATIVE_REMAP) { + if (!pw_pod_remap_data(SPA_POD_TYPE_STRUCT, message, size, &this->types)) { pw_log_error ("protocol-native %p: invalid message received %u for %u", this, opcode, id); - } else - pw_log_error("protocol-native %p: function %d not implemented on %u", this, - opcode, id); - + continue; + } + } + if (!demarshal[opcode].func(proxy, message, size)) { + pw_log_error ("protocol-native %p: invalid message received %u for %u", this, + opcode, id); + continue; + } } } } diff --git a/src/modules/module-protocol-native/protocol-native.c b/src/modules/module-protocol-native/protocol-native.c index b7e9e2d7d..7bf43fa72 100644 --- a/src/modules/module-protocol-native/protocol-native.c +++ b/src/modules/module-protocol-native/protocol-native.c @@ -30,12 +30,6 @@ #include "connection.h" -/** \cond */ - -typedef bool(*demarshal_func_t) (void *object, void *data, size_t size); - -/** \endcond */ - static void core_marshal_client_update(void *object, const struct spa_dict *props) { struct pw_proxy *proxy = object; @@ -442,7 +436,6 @@ static bool core_demarshal_create_node(void *object, void *data, size_t size) struct spa_dict props; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_STRING, &factory_name, SPA_POD_TYPE_STRING, &name, @@ -478,7 +471,6 @@ static bool core_demarshal_create_link(void *object, void *data, size_t size) struct spa_dict props; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &output_node_id, SPA_POD_TYPE_INT, &output_port_id, @@ -568,7 +560,6 @@ static bool registry_demarshal_bind(void *object, void *data, size_t size) uint32_t id, version, type, new_id; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &resource->client->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &id, SPA_POD_TYPE_ID, &type, @@ -689,7 +680,6 @@ static bool node_demarshal_info(void *object, void *data, size_t size) int i; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_LONG, &info.change_mask, SPA_POD_TYPE_STRING, &info.name, @@ -810,7 +800,6 @@ static bool link_demarshal_info(void *object, void *data, size_t size) struct pw_link_info info = { 0, }; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_LONG, &info.change_mask, SPA_POD_TYPE_INT, &info.output_node_id, @@ -831,7 +820,6 @@ static bool registry_demarshal_global(void *object, void *data, size_t size) uint32_t id, parent_id, permissions, type, version; if (!spa_pod_iter_struct(&it, data, size) || - !pw_pod_remap_data(SPA_POD_TYPE_STRUCT, data, size, &proxy->remote->types) || !spa_pod_iter_get(&it, SPA_POD_TYPE_INT, &id, SPA_POD_TYPE_INT, &parent_id, @@ -886,13 +874,13 @@ static const struct pw_core_methods pw_protocol_native_core_method_marshal = { &core_marshal_create_link }; -static const demarshal_func_t pw_protocol_native_core_method_demarshal[PW_CORE_METHOD_NUM] = { - &core_demarshal_update_types_server, - &core_demarshal_sync, - &core_demarshal_get_registry, - &core_demarshal_client_update, - &core_demarshal_create_node, - &core_demarshal_create_link +static const struct pw_protocol_native_demarshal pw_protocol_native_core_method_demarshal[PW_CORE_METHOD_NUM] = { + { &core_demarshal_update_types_server, 0, }, + { &core_demarshal_sync, 0, }, + { &core_demarshal_get_registry, 0, }, + { &core_demarshal_client_update, 0, }, + { &core_demarshal_create_node, PW_PROTOCOL_NATIVE_REMAP, }, + { &core_demarshal_create_link, PW_PROTOCOL_NATIVE_REMAP, } }; static const struct pw_core_events pw_protocol_native_core_event_marshal = { @@ -904,12 +892,12 @@ static const struct pw_core_events pw_protocol_native_core_event_marshal = { &core_marshal_info }; -static const demarshal_func_t pw_protocol_native_core_event_demarshal[PW_CORE_EVENT_NUM] = { - &core_demarshal_update_types_client, - &core_demarshal_done, - &core_demarshal_error, - &core_demarshal_remove_id, - &core_demarshal_info +static const struct pw_protocol_native_demarshal pw_protocol_native_core_event_demarshal[PW_CORE_EVENT_NUM] = { + { &core_demarshal_update_types_client, 0, }, + { &core_demarshal_done, 0, }, + { &core_demarshal_error, 0, }, + { &core_demarshal_remove_id, 0, }, + { &core_demarshal_info, 0, }, }; static const struct pw_protocol_marshal pw_protocol_native_core_marshal = { @@ -928,8 +916,8 @@ static const struct pw_registry_methods pw_protocol_native_registry_method_marsh ®istry_marshal_bind }; -static const demarshal_func_t pw_protocol_native_registry_method_demarshal[] = { - ®istry_demarshal_bind, +static const struct pw_protocol_native_demarshal pw_protocol_native_registry_method_demarshal[] = { + { ®istry_demarshal_bind, PW_PROTOCOL_NATIVE_REMAP, } }; static const struct pw_registry_events pw_protocol_native_registry_event_marshal = { @@ -938,9 +926,9 @@ static const struct pw_registry_events pw_protocol_native_registry_event_marshal ®istry_marshal_global_remove, }; -static const demarshal_func_t pw_protocol_native_registry_event_demarshal[] = { - ®istry_demarshal_global, - ®istry_demarshal_global_remove, +static const struct pw_protocol_native_demarshal pw_protocol_native_registry_event_demarshal[] = { + { ®istry_demarshal_global, PW_PROTOCOL_NATIVE_REMAP, }, + { ®istry_demarshal_global_remove, 0, } }; const struct pw_protocol_marshal pw_protocol_native_registry_marshal = { @@ -959,8 +947,8 @@ static const struct pw_module_events pw_protocol_native_module_event_marshal = { &module_marshal_info, }; -static const demarshal_func_t pw_protocol_native_module_event_demarshal[] = { - &module_demarshal_info, +static const struct pw_protocol_native_demarshal pw_protocol_native_module_event_demarshal[] = { + { &module_demarshal_info, 0, }, }; const struct pw_protocol_marshal pw_protocol_native_module_marshal = { @@ -977,8 +965,8 @@ static const struct pw_node_events pw_protocol_native_node_event_marshal = { &node_marshal_info, }; -static const demarshal_func_t pw_protocol_native_node_event_demarshal[] = { - &node_demarshal_info, +static const struct pw_protocol_native_demarshal pw_protocol_native_node_event_demarshal[] = { + { &node_demarshal_info, PW_PROTOCOL_NATIVE_REMAP, } }; static const struct pw_protocol_marshal pw_protocol_native_node_marshal = { @@ -995,8 +983,8 @@ static const struct pw_client_events pw_protocol_native_client_event_marshal = { &client_marshal_info, }; -static const demarshal_func_t pw_protocol_native_client_event_demarshal[] = { - &client_demarshal_info, +static const struct pw_protocol_native_demarshal pw_protocol_native_client_event_demarshal[] = { + { &client_demarshal_info, 0, }, }; static const struct pw_protocol_marshal pw_protocol_native_client_marshal = { @@ -1013,8 +1001,8 @@ static const struct pw_link_events pw_protocol_native_link_event_marshal = { &link_marshal_info, }; -static const demarshal_func_t pw_protocol_native_link_event_demarshal[] = { - &link_demarshal_info, +static const struct pw_protocol_native_demarshal pw_protocol_native_link_event_demarshal[] = { + { &link_demarshal_info, PW_PROTOCOL_NATIVE_REMAP, } }; static const struct pw_protocol_marshal pw_protocol_native_link_marshal = {