summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDavid Herrmann <dh.herrmann@gmail.com>2015-07-16 11:00:55 +0200
committerDavid Herrmann <dh.herrmann@gmail.com>2015-07-16 11:23:34 +0200
commit443a55981388f519fb6528a8ee042f9e69079b68 (patch)
tree8b35b0d76e9816e7e361fa2759f5a11efe5541ca
parent42921716a4d693cd72044b3626a7e87210d72eb3 (diff)
sd-bus: fix gvariant structure encoding
In gvariant, all fixed-size objects need to be sized a multiple of their alignment. If a structure has only fixed-size members, it is required to be fixed size itself. If you imagine a structure like (ty), you have an 8-byte member followed by an 1-byte member. Hence, the overall inner-size is 9. The alignment of the object is 8, though. Therefore, the specs mandates final padding after fixed-size structures, to make sure it's sized a multiple of its alignment (=> 16). On the gvariant decoder side, we already account for this in bus_gvariant_get_size(), as we apply overall padding to the size of the structure. Therefore, our decoder correctly skips such final padding when parsing fixed-size structure. On the gvariant encoder side, however, we don't account for this final padding. This patch fixes the structure and dict-entry encoders to properly place such padding at the end of non-uniform fixed-size structures. The problem can be easily seen by running: $ busctl --user monitor and $ busctl call --user org.freedesktop.systemd1 / org.foobar foobar "(ty)" 777 8 The monitor will fail to parse the message and print an error. With this patch applied, everything works fine again. This patch also adds a bunch of test-cases to force non-uniform structures with non-pre-aligned positions. Thanks to Jan Alexander Steffens <jan.steffens@gmail.com> for spotting this and narrowing it down to non-uniform gvariant structures. Fixes #597.
-rw-r--r--src/libsystemd/sd-bus/bus-message.c17
-rw-r--r--src/libsystemd/sd-bus/test-bus-marshal.c21
2 files changed, 36 insertions, 2 deletions
diff --git a/src/libsystemd/sd-bus/bus-message.c b/src/libsystemd/sd-bus/bus-message.c
index 983e2f62cd..18685be8ff 100644
--- a/src/libsystemd/sd-bus/bus-message.c
+++ b/src/libsystemd/sd-bus/bus-message.c
@@ -2161,6 +2161,7 @@ static int bus_message_close_variant(sd_bus_message *m, struct bus_container *c)
}
static int bus_message_close_struct(sd_bus_message *m, struct bus_container *c, bool add_offset) {
+ bool fixed_size = true;
size_t n_variable = 0;
unsigned i = 0;
const char *p;
@@ -2196,6 +2197,8 @@ static int bus_message_close_struct(sd_bus_message *m, struct bus_container *c,
/* We need to add an offset for each item that has a
* variable size and that is not the last one in the
* list */
+ if (r == 0)
+ fixed_size = false;
if (r == 0 && p[n] != 0)
n_variable++;
@@ -2207,7 +2210,19 @@ static int bus_message_close_struct(sd_bus_message *m, struct bus_container *c,
assert(c->need_offsets || n_variable == 0);
if (n_variable <= 0) {
- a = message_extend_body(m, 1, 0, add_offset, false);
+ int alignment = 1;
+
+ /* Structures with fixed-size members only have to be
+ * fixed-size themselves. But gvariant requires all fixed-size
+ * elements to be sized a multiple of their alignment. Hence,
+ * we must *always* add final padding after the last member so
+ * the overall size of the structure is properly aligned. */
+ if (fixed_size)
+ alignment = bus_gvariant_get_alignment(strempty(c->signature));
+
+ assert(alignment > 0);
+
+ a = message_extend_body(m, alignment, 0, add_offset, false);
if (!a)
return -ENOMEM;
} else {
diff --git a/src/libsystemd/sd-bus/test-bus-marshal.c b/src/libsystemd/sd-bus/test-bus-marshal.c
index a866a56179..73c6e41c85 100644
--- a/src/libsystemd/sd-bus/test-bus-marshal.c
+++ b/src/libsystemd/sd-bus/test-bus-marshal.c
@@ -131,6 +131,9 @@ int main(int argc, char *argv[]) {
r = sd_bus_message_append(m, "a{yv}", 2, 3, "s", "foo", 5, "s", "waldo");
assert_se(r >= 0);
+ r = sd_bus_message_append(m, "y(ty)y(yt)y", 8, 777, 7, 9, 77, 7777, 10);
+ assert_se(r >= 0);
+
r = sd_bus_message_append(m, "ba(ss)", 255, 3, "aaa", "1", "bbb", "2", "ccc", "3");
assert_se(r >= 0);
@@ -252,6 +255,22 @@ int main(int argc, char *argv[]) {
assert_se(v == 5);
assert_se(streq(y, "waldo"));
+ r = sd_bus_message_read(m, "y(ty)", &v, &u64, &u);
+ assert_se(r > 0);
+ assert_se(v == 8);
+ assert_se(u64 == 777);
+ assert_se(u == 7);
+
+ r = sd_bus_message_read(m, "y(yt)", &v, &u, &u64);
+ assert_se(r > 0);
+ assert_se(v == 9);
+ assert_se(u == 77);
+ assert_se(u64 == 7777);
+
+ r = sd_bus_message_read(m, "y", &v);
+ assert_se(r > 0);
+ assert_se(v == 10);
+
r = sd_bus_message_read(m, "ba(ss)", &boolean, 3, &x, &y, &a, &b, &c, &d);
assert_se(r > 0);
assert_se(boolean);
@@ -331,7 +350,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}");
+ 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);