[PATCH] malloc: Fix [BZ #22853] weak randomization on thread arenas.

[iripoll]

Hello,

>>
> 
> Alright, could you post a v2 with the inline functions change, without
> the test and with a proper commit message?
> 
> Thanks for you patience and for the work on moving this forward.
> 

Here is the the patch with all the comments and suggestions.

Regards,
Ismael and Hector.



 From 6230b07e0fc85856275f89c16a18b46f8ed23936 Mon Sep 17 00:00:00 2001
From: Ismael Ripoll <iripoll@disca.upv.es>
Date: Wed, 25 Mar 2020 12:36:24 +0100
Subject: [PATCH] Fix [BZ #22853] weak randomization on thread arenas.

Signed-off-by: Ismael Ripoll <iripoll@disca.upv.es>
Co-Authored-By: Hector Marco-Gisbert <hmarco@hmarco.org>
---
  malloc/arena.c | 38 +++++++++++++++++++++++++++++---------
  1 file changed, 29 insertions(+), 9 deletions(-)

diff --git a/malloc/arena.c b/malloc/arena.c
index cecdb7f4c4..623824088b 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -122,13 +122,23 @@ int __malloc_initialized = -1;
          ptr = arena_get2 ((size), NULL);				      \
    } while (0)

-/* find the heap and corresponding arena for a given ptr */
-
-#define heap_for_ptr(ptr) \
-  ((heap_info *) ((unsigned long) (ptr) & ~(HEAP_MAX_SIZE - 1)))
-#define arena_for_chunk(ptr) \
-  (chunk_main_arena (ptr) ? &main_arena : heap_for_ptr (ptr)->ar_ptr)
-
+/* find the heap and corresponding arena for a given ptr.  Note that
+   heap_info is not HEAP_MAX_SIZE aligned any more.  But a random
+   offset from the expected alignment, known by the process.  This way
+   it is fast to get the head of the area and be ASLR friendly.
+*/
+static unsigned long arena_rnd;
+static inline heap_info *
+heap_for_ptr (mchunkptr ptr)
+{
+  return (heap_info *) ((((uintptr_t) ptr - arena_rnd)
+                         & ~(HEAP_MAX_SIZE - 1)) | arena_rnd);
+}
+static inline struct malloc_state *
+arena_for_chunk (mchunkptr ptr)
+{
+  return chunk_main_arena (ptr) ? &main_arena : heap_for_ptr (ptr)->ar_ptr;
+}

  /**************************************************************************/

@@ -293,6 +303,11 @@ ptmalloc_init (void)

    __malloc_initialized = 0;

+  size_t pagesize = GLRO (dl_pagesize);
+  /* Get the entropy from the already existing ASLR. */
+  arena_rnd = ((unsigned long) & arena_rnd) & (HEAP_MAX_SIZE - 1) &
+              ~(pagesize - 1);
+
  #ifdef SHARED
    /* In case this libc copy is in a non-default namespace, never use brk.
       Likewise if dlopened from statically linked program.  */
@@ -490,6 +505,11 @@ new_heap (size_t size, size_t top_pad)
          {
            p2 = (char *) (((unsigned long) p1 + (HEAP_MAX_SIZE - 1))
                           & ~(HEAP_MAX_SIZE - 1));
+          /* The heap_info is at a random offset from the alignment to
+             HEAP_MAX_SIZE. */
+          p2 = (char *) ((unsigned long) p2 | arena_rnd);
+          if (p1 + HEAP_MAX_SIZE <= p2)
+            p2 -= HEAP_MAX_SIZE;
            ul = p2 - p1;
            if (ul)
              __munmap (p1, ul);
@@ -500,12 +520,12 @@ new_heap (size_t size, size_t top_pad)
        else
          {
            /* Try to take the chance that an allocation of only HEAP_MAX_SIZE
-             is already aligned. */
+             is already aligned to arena_rnd. */
            p2 = (char *) MMAP (0, HEAP_MAX_SIZE, PROT_NONE, MAP_NORESERVE);
            if (p2 == MAP_FAILED)
              return 0;

-          if ((unsigned long) p2 & (HEAP_MAX_SIZE - 1))
+          if (((unsigned long) p2 & (HEAP_MAX_SIZE - 1)) != arena_rnd)
              {
                __munmap (p2, HEAP_MAX_SIZE);
                return 0;
-- 
2.20.1