From 2a0958d2d011b2debdb266144754bb5cdc4da65d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 6 Jun 2014 18:30:01 +0200 Subject: [PATCH] sd-bus: don't allow creating message objects that are not attached to a bus It seems unnecessary to support this, and we rather should avoid allowing this at all, so that people don't program against this sloppily and we end up remarshalling all the time... --- src/libsystemd/sd-bus/bus-message.c | 28 ++++++++++++----------- src/libsystemd/sd-bus/test-bus-gvariant.c | 8 +++++-- src/libsystemd/sd-bus/test-bus-marshal.c | 11 ++++++--- src/libsystemd/sd-bus/test-bus-match.c | 14 ++++++++---- 4 files changed, 39 insertions(+), 22 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 4fcc69354..eaffa2d3a 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -136,8 +136,7 @@ static void message_free(sd_bus_message *m) { ioctl(m->bus->input_fd, KDBUS_CMD_FREE, &off); } - if (m->bus) - sd_bus_unref(m->bus); + sd_bus_unref(m->bus); if (m->free_fds) { close_many(m->fds, m->n_fds); @@ -373,6 +372,7 @@ int bus_message_from_header( struct bus_header *h; size_t a, label_sz; + assert(bus); assert(buffer || length <= 0); assert(fds || n_fds <= 0); assert(ret); @@ -426,10 +426,9 @@ int bus_message_from_header( m->creds.mask |= SD_BUS_CREDS_SELINUX_CONTEXT; } - if (bus) - m->bus = sd_bus_ref(bus); - + m->bus = sd_bus_ref(bus); *ret = m; + return 0; } @@ -489,6 +488,8 @@ fail: static sd_bus_message *message_new(sd_bus *bus, uint8_t type) { sd_bus_message *m; + assert(bus); + m = malloc0(ALIGN(sizeof(sd_bus_message)) + sizeof(struct bus_header)); if (!m) return NULL; @@ -500,9 +501,7 @@ static sd_bus_message *message_new(sd_bus *bus, uint8_t type) { m->header->version = bus ? bus->message_version : 1; m->allow_fds = !bus || bus->can_fds || (bus->state != BUS_HELLO && bus->state != BUS_RUNNING); m->root_container.need_offsets = BUS_MESSAGE_IS_GVARIANT(m); - - if (bus) - m->bus = sd_bus_ref(bus); + m->bus = sd_bus_ref(bus); return m; } @@ -517,7 +516,8 @@ _public_ int sd_bus_message_new_signal( sd_bus_message *t; int r; - assert_return(!bus || bus->state != BUS_UNSET, -ENOTCONN); + assert_return(bus, -ENOTCONN); + assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(object_path_is_valid(path), -EINVAL); assert_return(interface_name_is_valid(interface), -EINVAL); assert_return(member_name_is_valid(member), -EINVAL); @@ -558,7 +558,8 @@ _public_ int sd_bus_message_new_method_call( sd_bus_message *t; int r; - assert_return(!bus || bus->state != BUS_UNSET, -ENOTCONN); + assert_return(bus, -ENOTCONN); + assert_return(bus->state != BUS_UNSET, -ENOTCONN); assert_return(!destination || service_name_is_valid(destination), -EINVAL); assert_return(object_path_is_valid(path), -EINVAL); assert_return(!interface || interface_name_is_valid(interface), -EINVAL); @@ -607,7 +608,7 @@ static int message_new_reply( assert_return(call, -EINVAL); assert_return(call->sealed, -EPERM); assert_return(call->header->type == SD_BUS_MESSAGE_METHOD_CALL, -EINVAL); - assert_return(!call->bus || call->bus->state != BUS_UNSET, -ENOTCONN); + assert_return(call->bus->state != BUS_UNSET, -ENOTCONN); assert_return(m, -EINVAL); t = message_new(call->bus, type); @@ -742,6 +743,7 @@ int bus_message_new_synthetic_error( sd_bus_message *t; int r; + assert(bus); assert(sd_bus_error_is_set(e)); assert(m); @@ -2784,7 +2786,7 @@ int bus_message_seal(sd_bus_message *m, uint64_t cookie, usec_t timeout) { /* If this is something we can send as memfd, then let's seal the memfd now. Note that we can send memfds as payload only for directed messages, and not for broadcasts. */ - if (m->destination && m->bus && m->bus->use_memfd) { + if (m->destination && m->bus->use_memfd) { MESSAGE_FOREACH_PART(part, i, m) if (part->memfd >= 0 && !part->sealed && (part->size > MEMFD_MIN_SIZE || m->bus->use_memfd < 0)) { uint64_t sz; @@ -5045,7 +5047,7 @@ int bus_message_parse_fields(sd_bus_message *m) { r = message_peek_field_string(m, service_name_is_valid, &ri, item_size, &m->sender); - if (r >= 0 && m->sender[0] == ':' && m->bus && m->bus->bus_client && !m->bus->is_kernel) { + if (r >= 0 && m->sender[0] == ':' && m->bus->bus_client && !m->bus->is_kernel) { m->creds.unique_name = (char*) m->sender; m->creds.mask |= SD_BUS_CREDS_UNIQUE_NAME & m->bus->creds_mask; } diff --git a/src/libsystemd/sd-bus/test-bus-gvariant.c b/src/libsystemd/sd-bus/test-bus-gvariant.c index f2c1fc125..922685882 100644 --- a/src/libsystemd/sd-bus/test-bus-gvariant.c +++ b/src/libsystemd/sd-bus/test-bus-gvariant.c @@ -134,8 +134,12 @@ static void test_marshal(void) { _cleanup_bus_unref_ sd_bus *bus = NULL; _cleanup_free_ void *blob; size_t sz; + int r; + + r = sd_bus_open_system(&bus); + if (r < 0) + exit(EXIT_TEST_SKIP); - assert_se(sd_bus_open_system(&bus) >= 0); bus->message_version = 2; /* dirty hack to enable gvariant*/ assert_se(sd_bus_message_new_method_call(bus, &m, "a.service.name", "/an/object/path/which/is/really/really/long/so/that/we/hit/the/eight/bit/boundary/by/quite/some/margin/to/test/this/stuff/that/it/really/works", "an.interface.name", "AMethodName") >= 0); @@ -175,7 +179,7 @@ static void test_marshal(void) { assert_se(bus_message_get_blob(m, &blob, &sz) >= 0); - assert_se(bus_message_from_malloc(NULL, blob, sz, NULL, 0, NULL, NULL, &n) >= 0); + assert_se(bus_message_from_malloc(bus, blob, sz, NULL, 0, NULL, NULL, &n) >= 0); blob = NULL; assert_se(bus_message_dump(n, NULL, true) >= 0); diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 2be4de571..953211252 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/src/libsystemd/sd-bus/test-bus-marshal.c @@ -93,8 +93,13 @@ int main(int argc, char *argv[]) { _cleanup_free_ char *first = NULL, *second = NULL, *third = NULL; _cleanup_fclose_ FILE *ms = NULL; size_t first_size = 0, second_size = 0, third_size = 0; + _cleanup_bus_unref_ sd_bus *bus = NULL; - r = sd_bus_message_new_method_call(NULL, &m, "foobar.waldo", "/", "foobar.waldo", "Piep"); + r = sd_bus_default_system(&bus); + if (r < 0) + return EXIT_TEST_SKIP; + + r = sd_bus_message_new_method_call(bus, &m, "foobar.waldo", "/", "foobar.waldo", "Piep"); assert_se(r >= 0); r = sd_bus_message_append(m, ""); @@ -193,7 +198,7 @@ int main(int argc, char *argv[]) { m = sd_bus_message_unref(m); - r = bus_message_from_malloc(NULL, buffer, sz, NULL, 0, NULL, NULL, &m); + r = bus_message_from_malloc(bus, buffer, sz, NULL, 0, NULL, NULL, &m); assert_se(r >= 0); bus_message_dump(m, stdout, true); @@ -266,7 +271,7 @@ int main(int argc, char *argv[]) { r = sd_bus_message_peek_type(m, NULL, NULL); assert_se(r == 0); - r = sd_bus_message_new_method_call(NULL, ©, "foobar.waldo", "/", "foobar.waldo", "Piep"); + r = sd_bus_message_new_method_call(bus, ©, "foobar.waldo", "/", "foobar.waldo", "Piep"); assert_se(r >= 0); r = sd_bus_message_rewind(m, true); diff --git a/src/libsystemd/sd-bus/test-bus-match.c b/src/libsystemd/sd-bus/test-bus-match.c index a62de502f..6c5d35b3d 100644 --- a/src/libsystemd/sd-bus/test-bus-match.c +++ b/src/libsystemd/sd-bus/test-bus-match.c @@ -80,13 +80,19 @@ static int match_add(sd_bus_slot *slots, struct bus_match_node *root, const char } int main(int argc, char *argv[]) { - struct bus_match_node root; + struct bus_match_node root = { + .type = BUS_MATCH_ROOT, + }; + _cleanup_bus_message_unref_ sd_bus_message *m = NULL; + _cleanup_bus_unref_ sd_bus *bus = NULL; enum bus_match_node_type i; sd_bus_slot slots[15]; + int r; - zero(root); - root.type = BUS_MATCH_ROOT; + r = sd_bus_open_system(&bus); + if (r < 0) + return EXIT_TEST_SKIP; assert_se(match_add(slots, &root, "arg2='wal\\'do',sender='foo',type='signal',interface='bar.x',", 1) >= 0); assert_se(match_add(slots, &root, "arg2='wal\\'do2',sender='foo',type='signal',interface='bar.x',", 2) >= 0); @@ -105,7 +111,7 @@ int main(int argc, char *argv[]) { bus_match_dump(&root, 0); - assert_se(sd_bus_message_new_signal(NULL, &m, "/foo/bar", "bar.x", "waldo") >= 0); + assert_se(sd_bus_message_new_signal(bus, &m, "/foo/bar", "bar.x", "waldo") >= 0); assert_se(sd_bus_message_append(m, "ssss", "one", "two", "/prefix/three", "prefix.four") >= 0); assert_se(bus_message_seal(m, 1, 0) >= 0); -- 2.30.2