gvariant: Realign data on construction if it’s not properly aligned

Otherwise the GVariant would later fail internal alignment checks,
aborting the program.

If unaligned data is provided to (for example)
g_variant_new_from_data(), it will copy the data into a new aligned
allocation. This is slow, but better than crashing. If callers want
better performance, they should provide aligned data in their call, and
it will not be copied or reallocated.

Includes a unit test.

Signed-off-by: Philip Withnall <withnall@endlessm.com>

https://gitlab.gnome.org/GNOME/glib/issues/1342

glib/gvariant-core.c

@@ -506,6 +506,10 @@ g_variant_alloc (const GVariantType *type,
  *
  * A reference is taken on @bytes.
  *
+ * The data in @bytes must be aligned appropriately for the @type being loaded.
+ * Otherwise this function will internally create a copy of the memory (since
+ * GLib 2.60) or (in older versions) fail and exit the process.
+ *
  * Returns: (transfer none): a new #GVariant with a floating reference
  *
  * Since: 2.36
@@ -518,14 +522,50 @@ g_variant_new_from_bytes (const GVariantType *type,
   GVariant *value;
   guint alignment;
   gsize size;
+  GBytes *owned_bytes = NULL;
 
   value = g_variant_alloc (type, TRUE, trusted);
 
-  value->contents.serialised.bytes = g_bytes_ref (bytes);
-
   g_variant_type_info_query (value->type_info,
                              &alignment, &size);
 
+  /* Ensure the alignment is correct. This is a huge performance hit if it’s
+   * not correct, but that’s better than aborting if a caller provides data
+   * with the wrong alignment (which is likely to happen very occasionally, and
+   * only cause an abort on some architectures — so is unlikely to be caught
+   * in testing). Callers can always actively ensure they use the correct
+   * alignment to avoid the performance hit. */
+  if ((alignment & (gsize) g_bytes_get_data (bytes, NULL)) != 0)
+    {
+#ifdef HAVE_POSIX_MEMALIGN
+      gpointer aligned_data = NULL;
+      gsize aligned_size = g_bytes_get_size (bytes);
+
+      /* posix_memalign() requires the alignment to be a multiple of
+       * sizeof(void*), and a power of 2. See g_variant_type_info_query() for
+       * details on the alignment format. */
+      if (posix_memalign (&aligned_data, MAX (sizeof (void *), alignment + 1),
+                          aligned_size) != 0)
+        g_error ("posix_memalign failed");
+
+      memcpy (aligned_data, g_bytes_get_data (bytes, NULL), aligned_size);
+
+      bytes = owned_bytes = g_bytes_new_with_free_func (aligned_data,
+                                                        aligned_size,
+                                                        free, aligned_data);
+      aligned_data = NULL;
+#else
+      /* NOTE: there may be platforms that lack posix_memalign() and also
+       * have malloc() that returns non-8-aligned.  if so, we need to try
+       * harder here.
+       */
+      bytes = owned_bytes = g_bytes_new (g_bytes_get_data (bytes, NULL),
+                                         g_bytes_get_size (bytes));
+#endif
+    }
+
+  value->contents.serialised.bytes = g_bytes_ref (bytes);
+
   if (size && g_bytes_get_size (bytes) != size)
     {
       /* Creating a fixed-sized GVariant with a bytes of the wrong
@@ -543,6 +583,8 @@ g_variant_new_from_bytes (const GVariantType *type,
       value->contents.serialised.data = g_bytes_get_data (bytes, &value->size);
     }
 
+  g_clear_pointer (&owned_bytes, g_bytes_unref);
+
   return value;
 }
 

glib/gvariant.c

@@ -307,6 +307,11 @@
  * Constructs a new trusted #GVariant instance from the provided data.
  * This is used to implement g_variant_new_* for all the basic types.
  *
+ * Note: @data must be backed by memory that is aligned appropriately for the
+ * @type being loaded. Otherwise this function will internally create a copy of
+ * the memory (since GLib 2.60) or (in older versions) fail and exit the
+ * process.
+ *
  * Returns: a new floating #GVariant
  */
 static GVariant *
@@ -5986,6 +5991,11 @@ g_variant_byteswap (GVariant *value)
  * needed.  The exact time of this call is unspecified and might even be
  * before this function returns.
  *
+ * Note: @data must be backed by memory that is aligned appropriately for the
+ * @type being loaded. Otherwise this function will internally create a copy of
+ * the memory (since GLib 2.60) or (in older versions) fail and exit the
+ * process.
+ *
  * Returns: (transfer none): a new floating #GVariant of type @type
  *
  * Since: 2.24

glib/tests/gvariant.c

@@ -4965,6 +4965,51 @@ test_normal_checking_empty_object_path (void)
   g_variant_unref (variant);
 }
 
+/* Test that constructing a #GVariant from data which is not correctly aligned
+ * for the variant type is OK, by loading a variant from data at various offsets
+ * which are aligned and unaligned. When unaligned, a slow construction path
+ * should be taken. */
+static void
+test_unaligned_construction (void)
+{
+  const guint8 data[] = {
+    0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+    0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+  };
+  GVariant *variant = NULL;
+  GVariant *normal_variant = NULL;
+  gsize i, offset;
+  const struct {
+    const GVariantType *type;
+    gsize size;
+    gsize max_offset;
+  } vectors[] = {
+    { G_VARIANT_TYPE_UINT64, sizeof (guint64), sizeof (guint64) },
+    { G_VARIANT_TYPE_UINT32, sizeof (guint32), sizeof (guint32) },
+    { G_VARIANT_TYPE_UINT16, sizeof (guint16), sizeof (guint16) },
+    { G_VARIANT_TYPE_BYTE, sizeof (guint8), 3 },
+  };
+
+  G_STATIC_ASSERT (sizeof (guint64) * 2 <= sizeof (data));
+
+  for (i = 0; i < G_N_ELEMENTS (vectors); i++)
+    {
+      for (offset = 0; offset < vectors[i].max_offset; offset++)
+        {
+          variant = g_variant_new_from_data (vectors[i].type, data + offset,
+                                             vectors[i].size,
+                                             FALSE, NULL, NULL);
+          g_assert_nonnull (variant);
+
+          normal_variant = g_variant_get_normal_form (variant);
+          g_assert_nonnull (normal_variant);
+
+          g_variant_unref (normal_variant);
+          g_variant_unref (variant);
+        }
+    }
+}
+
 int
 main (int argc, char **argv)
 {
@@ -5045,5 +5090,8 @@ main (int argc, char **argv)
   g_test_add_func ("/gvariant/recursion-limits/array-in-variant",
                    test_recursion_limits_array_in_variant);
 
+  g_test_add_func ("/gvariant/unaligned-construction",
+                   test_unaligned_construction);
+
   return g_test_run ();
 }