[glib/rc-align: 33/34] Align the reference counted allocations

[Emmanuele Bassi <ebassi src gnome org>]

commit 7ec0328c3feb7d2c04c450ee2be8ac41f3954e22
Author: Emmanuele Bassi <ebassi gnome org>
Date:   Wed Nov 14 14:43:23 2018 +0000

    Align the reference counted allocations
    
    We need stronger alignment guarantees for the memory allocations done
    through g_rc_box_alloc_full(): while the passed block size may be
    aligned, we're not aligning the private data size; this means the
    overall allocation may become unaligned, and this could raise issues
    when we use the private data size as an offset to access the reference
    count.
    
    Fixes: #1581

 glib/garcbox.c       | 10 ++++----
 glib/grcbox.c        | 65 +++++++++++++++++++++++++++++++++++++++++++---------
 glib/grcboxprivate.h |  5 ++++
 3 files changed, 65 insertions(+), 15 deletions(-)
---
diff --git a/glib/garcbox.c b/glib/garcbox.c
index 9c1bd8fe5..02c9266aa 100644
--- a/glib/garcbox.c
+++ b/glib/garcbox.c
@@ -177,7 +177,7 @@ g_atomic_rc_box_alloc (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, TRUE, FALSE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, FALSE);
 }
 
 /**
@@ -201,7 +201,7 @@ g_atomic_rc_box_alloc0 (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, TRUE, TRUE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, TRUE);
 }
 
 /**
@@ -262,7 +262,7 @@ gpointer
   g_return_val_if_fail (block_size > 0, NULL);
   g_return_val_if_fail (mem_block != NULL, NULL);
 
-  res = g_rc_box_alloc_full (block_size, TRUE, FALSE);
+  res = g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, TRUE, FALSE);
   memcpy (res, mem_block, block_size);
 
   return res;
@@ -339,13 +339,15 @@ g_atomic_rc_box_release_full (gpointer       mem_block,
 
   if (g_atomic_ref_count_dec (&real_box->ref_count))
     {
+      char *real_mem = (char *) real_box - real_box->private_offset;
+
       TRACE (GLIB_RCBOX_RELEASE (mem_block, 1));
 
       if (clear_func != NULL)
         clear_func (mem_block);
 
       TRACE (GLIB_RCBOX_FREE (mem_block));
-      g_free (real_box);
+      g_free (real_mem);
     }
 }
 
diff --git a/glib/grcbox.c b/glib/grcbox.c
index f31db78ab..d4b52d2f7 100644
--- a/glib/grcbox.c
+++ b/glib/grcbox.c
@@ -162,25 +162,46 @@
  * Since: 2.58.
  */
 
-#define G_RC_BOX(p)             (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE)
-
 /* We use the same alignment as GTypeInstance and GNU libc's malloc */
-#define STRUCT_ALIGNMENT        (2 * sizeof (gsize))
 #define ALIGN_STRUCT(offset)    ((offset + (STRUCT_ALIGNMENT - 1)) & -STRUCT_ALIGNMENT)
 
+#define G_RC_BOX(p)             (GRcBox *) (((char *) (p)) - G_RC_BOX_SIZE)
+
 gpointer
 g_rc_box_alloc_full (gsize    block_size,
+                     gsize    alignment,
                      gboolean atomic,
                      gboolean clear)
 {
-  /* sizeof GArcBox == sizeof GRcBox */
+  /* We don't do an (atomic ? G_ARC_BOX_SIZE : G_RC_BOX_SIZE) check, here
+   * because we have a static assertion that sizeof(GArcBox) == sizeof(GRcBox)
+   * inside grcboxprivate.h, and we don't want the compiler to unnecessarily
+   * warn about both branches of the conditional yielding identical results
+   */
   gsize private_size = G_ARC_BOX_SIZE;
+  gsize private_offset = 0;
   gsize real_size;
   char *allocated;
 
-  g_assert (block_size < (G_MAXSIZE - G_ARC_BOX_SIZE));
+  /* We need to ensure that the private data is aligned */
+  if (private_size % alignment != 0)
+    {
+      private_offset = private_size % alignment;
+      private_size += (alignment - private_offset);
+    }
+
+  g_assert (block_size < (G_MAXSIZE - private_size));
   real_size = private_size + block_size;
 
+  /* The real allocated size must be a multiple of alignment, to
+   * maintain the alignment of block_size
+   */
+  if (real_size % alignment != 0)
+    {
+      gsize offset = real_size % alignment;
+      real_size += (alignment - offset);
+    }
+
 #ifdef ENABLE_VALGRIND
   if (RUNNING_ON_VALGRIND)
     {
@@ -214,8 +235,18 @@ g_rc_box_alloc_full (gsize    block_size,
 
   if (atomic)
     {
-      GArcBox *real_box = (GArcBox *) allocated;
+      /* We leave the alignment padding at the top of the allocation,
+       * so we have an in memory layout of:
+       *
+       *  |[ offset ][ sizeof(GArcBox) ]||[ block_size ]|
+       */
+      GArcBox *real_box = (GArcBox *) (allocated + private_offset);
+      /* Store the real size */
       real_box->mem_size = block_size;
+      /* Store the alignment offset, to be used when freeing the
+       * allocated block
+       */
+      real_box->private_offset = private_offset;
 #ifndef G_DISABLE_ASSERT
       real_box->magic = G_BOX_MAGIC;
 #endif
@@ -223,8 +254,18 @@ g_rc_box_alloc_full (gsize    block_size,
     }
   else
     {
-      GRcBox *real_box = (GRcBox *) allocated;
+      /* We leave the alignment padding at the top of the allocation,
+       * so we have an in memory layout of:
+       *
+       *  |[ offset ][ sizeof(GRcBox) ]||[ block_size ]|
+       */
+      GRcBox *real_box = (GRcBox *) (allocated + private_offset);
+      /* Store the real size */
       real_box->mem_size = block_size;
+      /* Store the alignment offset, to be used when freeing the
+       * allocated block
+       */
+      real_box->private_offset = private_offset;
 #ifndef G_DISABLE_ASSERT
       real_box->magic = G_BOX_MAGIC;
 #endif
@@ -255,7 +296,7 @@ g_rc_box_alloc (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, FALSE, FALSE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, FALSE);
 }
 
 /**
@@ -279,7 +320,7 @@ g_rc_box_alloc0 (gsize block_size)
 {
   g_return_val_if_fail (block_size > 0, NULL);
 
-  return g_rc_box_alloc_full (block_size, FALSE, TRUE);
+  return g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, TRUE);
 }
 
 /**
@@ -339,7 +380,7 @@ gpointer
   g_return_val_if_fail (block_size > 0, NULL);
   g_return_val_if_fail (mem_block != NULL, NULL);
 
-  res = g_rc_box_alloc_full (block_size, FALSE, FALSE);
+  res = g_rc_box_alloc_full (block_size, STRUCT_ALIGNMENT, FALSE, FALSE);
   memcpy (res, mem_block, block_size);
 
   return res;
@@ -416,13 +457,15 @@ g_rc_box_release_full (gpointer       mem_block,
 
   if (g_ref_count_dec (&real_box->ref_count))
     {
+      char *real_mem = (char *) real_box - real_box->private_offset;
+
       TRACE (GLIB_RCBOX_RELEASE (mem_block, 0));
 
       if (clear_func != NULL)
         clear_func (mem_block);
 
       TRACE (GLIB_RCBOX_FREE (mem_block));
-      g_free (real_box);
+      g_free (real_mem);
     }
 }
 
diff --git a/glib/grcboxprivate.h b/glib/grcboxprivate.h
index 8b0d8dd4e..36c037606 100644
--- a/glib/grcboxprivate.h
+++ b/glib/grcboxprivate.h
@@ -27,6 +27,7 @@ typedef struct {
   grefcount ref_count;
 
   gsize mem_size;
+  gsize private_offset;
 
 #ifndef G_DISABLE_ASSERT
   /* A "magic" number, used to perform additional integrity
@@ -40,6 +41,7 @@ typedef struct {
   gatomicrefcount ref_count;
 
   gsize mem_size;
+  gsize private_offset;
 
 #ifndef G_DISABLE_ASSERT
   guint32 magic;
@@ -51,10 +53,13 @@ typedef struct {
 /* Keep the two refcounted boxes identical in size */
 G_STATIC_ASSERT (sizeof (GRcBox) == sizeof (GArcBox));
 
+#define STRUCT_ALIGNMENT (2 * sizeof (gsize))
+
 #define G_RC_BOX_SIZE sizeof (GRcBox)
 #define G_ARC_BOX_SIZE sizeof (GArcBox)
 
 gpointer        g_rc_box_alloc_full     (gsize    block_size,
+                                         gsize    alignment,
                                          gboolean atomic,
                                          gboolean clear);