From a388569f609017430133c1c6584eca34cbff3aa3 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 28 Jul 2015 16:46:51 +0200 Subject: sd-bus: don't assert() on valid signatures Right now sd_bus_message_skip() will abort execution if passed a signature of the unary type "()". Regardless whether this should be supported or not, we really must not abort. Drop the incorrect assertion and add a test-case for this. --- src/libsystemd/sd-bus/bus-message.c | 1 - src/libsystemd/sd-bus/test-bus-marshal.c | 8 +++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 18685be8ff..081f39d83b 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -4756,7 +4756,6 @@ _public_ int sd_bus_message_skip(sd_bus_message *m, const char *types) { r = sd_bus_message_skip(m, s); if (r < 0) return r; - assert(r != 0); r = sd_bus_message_exit_container(m); if (r < 0) diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c index 59deaea89f..b203707f27 100644 --- a/src/libsystemd/sd-bus/test-bus-marshal.c +++ b/src/libsystemd/sd-bus/test-bus-marshal.c @@ -134,6 +134,9 @@ int main(int argc, char *argv[]) { r = sd_bus_message_append(m, "y(ty)y(yt)y", 8, 777ULL, 7, 9, 77, 7777ULL, 10); assert_se(r >= 0); + r = sd_bus_message_append(m, "()"); + assert_se(r >= 0); + r = sd_bus_message_append(m, "ba(ss)", 255, 3, "aaa", "1", "bbb", "2", "ccc", "3"); assert_se(r >= 0); @@ -271,6 +274,9 @@ int main(int argc, char *argv[]) { assert_se(r > 0); assert_se(v == 10); + r = sd_bus_message_read(m, "()"); + assert_se(r > 0); + r = sd_bus_message_read(m, "ba(ss)", &boolean, 3, &x, &y, &a, &b, &c, &d); assert_se(r > 0); assert_se(boolean); @@ -350,7 +356,7 @@ int main(int argc, char *argv[]) { assert_se(sd_bus_message_verify_type(m, 'a', "{yv}") > 0); - r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y"); + r = sd_bus_message_skip(m, "a{yv}y(ty)y(yt)y()"); assert_se(r >= 0); assert_se(sd_bus_message_verify_type(m, 'b', NULL) > 0); -- cgit v1.2.3-54-g00ecf From 9a8fb5ae172f03c26618b8a75478417bec8c0dd2 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 28 Jul 2015 18:13:33 +0200 Subject: sd-bus: drop redundant code If c->item_size is 0, the next item to parse in a structure is empty. However, this also implies that the signature must be empty. The latter case is already handled just fine by enter_struct_or_dict_entry() so there is no reason to handle the same case in the caller. --- src/libsystemd/sd-bus/bus-message.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index 081f39d83b..c8806d516a 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -3954,12 +3954,6 @@ static int enter_struct_or_dict_entry( if (r < 0) return r; - } else if (c->item_size <= 0) { - - /* gvariant empty struct */ - *item_size = 0; - *offsets = NULL; - *n_offsets = 0; } else /* gvariant with contents */ return build_struct_offsets(m, contents, c->item_size, item_size, offsets, n_offsets); -- cgit v1.2.3-54-g00ecf From e53d21d00740a1dfc2090cb5f1ac29b11b2baa27 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Tue, 28 Jul 2015 18:16:16 +0200 Subject: sd-bus: fix marshaling of unary type The unary type has a fixed size of 1 in gvariant. Make sure we properly encode it as such. Right now, we encode/decode it as empty sequence. --- src/libsystemd/sd-bus/bus-gvariant.c | 21 +++++++++++++-------- src/libsystemd/sd-bus/bus-message.c | 26 ++++++++++++++++++++++++-- src/libsystemd/sd-bus/test-bus-gvariant.c | 2 +- 3 files changed, 38 insertions(+), 11 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-gvariant.c b/src/libsystemd/sd-bus/bus-gvariant.c index 2d18a4e6c1..402d43d66d 100644 --- a/src/libsystemd/sd-bus/bus-gvariant.c +++ b/src/libsystemd/sd-bus/bus-gvariant.c @@ -75,14 +75,19 @@ int bus_gvariant_get_size(const char *signature) { case SD_BUS_TYPE_STRUCT_BEGIN: case SD_BUS_TYPE_DICT_ENTRY_BEGIN: { - char t[n-1]; - - memcpy(t, p + 1, n - 2); - t[n - 2] = 0; - - r = bus_gvariant_get_size(t); - if (r < 0) - return r; + if (n == 2) { + /* unary type () has fixed size of 1 */ + r = 1; + } else { + char t[n-1]; + + memcpy(t, p + 1, n - 2); + t[n - 2] = 0; + + r = bus_gvariant_get_size(t); + if (r < 0) + return r; + } sum += r; break; diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index c8806d516a..c37a44493a 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -2209,7 +2209,14 @@ static int bus_message_close_struct(sd_bus_message *m, struct bus_container *c, assert(!c->need_offsets || i == c->n_offsets); assert(c->need_offsets || n_variable == 0); - if (n_variable <= 0) { + if (isempty(c->signature)) { + /* The unary type is encoded as fixed 1 byte padding */ + a = message_extend_body(m, 1, 1, add_offset, false); + if (!a) + return -ENOMEM; + + *a = 0; + } else if (n_variable <= 0) { int alignment = 1; /* Structures with fixed-size members only have to be @@ -3814,6 +3821,14 @@ static int build_struct_offsets( assert(n_offsets); if (isempty(signature)) { + /* Unary type is encoded as *fixed* 1 byte padding */ + r = message_peek_body(m, &m->rindex, 1, 1, &q); + if (r < 0) + return r; + + if (*(uint8_t *) q != 0) + return -EBADMSG; + *item_size = 0; *offsets = NULL; *n_offsets = 0; @@ -4140,7 +4155,14 @@ _public_ int sd_bus_message_enter_container(sd_bus_message *m, w->before = before; w->begin = m->rindex; - w->end = m->rindex + c->item_size; + + /* Unary type has fixed size of 1, but virtual size of 0 */ + if (BUS_MESSAGE_IS_GVARIANT(m) && + type == SD_BUS_TYPE_STRUCT && + isempty(signature)) + w->end = m->rindex + 0; + else + w->end = m->rindex + c->item_size; w->array_size = array_size; w->item_size = item_size; diff --git a/src/libsystemd/sd-bus/test-bus-gvariant.c b/src/libsystemd/sd-bus/test-bus-gvariant.c index 9b7dd2e499..b078bdc5f6 100644 --- a/src/libsystemd/sd-bus/test-bus-gvariant.c +++ b/src/libsystemd/sd-bus/test-bus-gvariant.c @@ -59,7 +59,7 @@ static void test_bus_gvariant_is_fixed_size(void) { static void test_bus_gvariant_get_size(void) { assert_se(bus_gvariant_get_size("") == 0); - assert_se(bus_gvariant_get_size("()") == 0); + assert_se(bus_gvariant_get_size("()") == 1); assert_se(bus_gvariant_get_size("y") == 1); assert_se(bus_gvariant_get_size("u") == 4); assert_se(bus_gvariant_get_size("b") == 1); -- cgit v1.2.3-54-g00ecf From 03ebf5e89bf63a32a2ffe253bec9e506f6917c80 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Wed, 29 Jul 2015 15:58:25 +0200 Subject: sd-bus: fix encoding/decoding gvariant root container The gvariant root container contains a 'variant' at the end, which embeds the whole message body. This variant *must* contain a structure so we are compatible to dbus1. Otherwise, it could encode at most 1 type, instead of a full signature. Our gvariant message parser already parses the variant-content as a structure, so we're mostly good. However, it does *not* include the opening and closing parantheses, nor does it parse them. This patch fixes the decoder to verify a message contains the parantheses, and also make the encoder add those parantheses into the marshaled message. --- src/libsystemd/sd-bus/bus-message.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c index c37a44493a..006e4a2b58 100644 --- a/src/libsystemd/sd-bus/bus-message.c +++ b/src/libsystemd/sd-bus/bus-message.c @@ -2906,18 +2906,20 @@ static int bus_message_close_header(sd_bus_message *m) { signature = strempty(m->root_container.signature); l = strlen(signature); - sz = bus_gvariant_determine_word_size(sizeof(struct bus_header) + ALIGN8(m->fields_size) + m->body_size + 1 + l, 1); - d = message_extend_body(m, 1, 1 + l + sz, false, true); + sz = bus_gvariant_determine_word_size(sizeof(struct bus_header) + ALIGN8(m->fields_size) + m->body_size + 1 + l + 2, 1); + d = message_extend_body(m, 1, 1 + l + 2 + sz, false, true); if (!d) return -ENOMEM; *(uint8_t*) d = 0; - memcpy((uint8_t*) d + 1, signature, l); + *((uint8_t*) d + 1) = SD_BUS_TYPE_STRUCT_BEGIN; + memcpy((uint8_t*) d + 2, signature, l); + *((uint8_t*) d + 1 + l + 1) = SD_BUS_TYPE_STRUCT_END; - bus_gvariant_write_word_le((uint8_t*) d + 1 + l, sz, sizeof(struct bus_header) + m->fields_size); + bus_gvariant_write_word_le((uint8_t*) d + 1 + l + 2, sz, sizeof(struct bus_header) + m->fields_size); m->footer = d; - m->footer_accessible = 1 + l + sz; + m->footer_accessible = 1 + l + 2 + sz; } else { m->header->dbus1.fields_size = m->fields_size; m->header->dbus1.body_size = m->body_size; @@ -5179,11 +5181,21 @@ int bus_message_parse_fields(sd_bus_message *m) { return -EBADMSG; if (*p == 0) { + size_t l; char *c; - /* We found the beginning of the signature string, yay! */ + /* We found the beginning of the signature + * string, yay! We require the body to be a + * structure, so verify it and then strip the + * opening/closing brackets. */ - c = strndup(p + 1, ((char*) m->footer + m->footer_accessible) - p - (1 + sz)); + l = ((char*) m->footer + m->footer_accessible) - p - (1 + sz); + if (l < 2 || + p[1] != SD_BUS_TYPE_STRUCT_BEGIN || + p[1 + l - 1] != SD_BUS_TYPE_STRUCT_END) + return -EBADMSG; + + c = strndup(p + 1 + 1, l - 2); if (!c) return -ENOMEM; -- cgit v1.2.3-54-g00ecf