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.

1 files changed, 16 insertions, 1 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 {