[v3,25/32] elf: Move most of the _dl_find_object data to the protected heap

Message ID a225e2d43b0e1d2e463bf6ad666ec9c1a165fdd2.1701944612.git.fweimer@redhat.com
State New
Headers
Series RELRO linkmaps |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-still_applies warning Patch no longer applies to master

Commit Message

Florian Weimer Dec. 7, 2023, 10:32 a.m. UTC
  The heap is mostly read-only by design, so allocation padding is
no longer required.  The protected heap is not visible to malloc,
so it's not necessary to deallocate the allocations during
__libc_freeres anymore.
---
 elf/dl-find_object.c  | 94 ++++++++-----------------------------------
 elf/dl-find_object.h  |  3 --
 elf/dl-libc_freeres.c |  2 -
 3 files changed, 16 insertions(+), 83 deletions(-)
  

Comments

Joseph Myers Feb. 28, 2024, 7:06 p.m. UTC | #1
On Thu, 7 Dec 2023, Florian Weimer wrote:

> The heap is mostly read-only by design, so allocation padding is
> no longer required.  The protected heap is not visible to malloc,
> so it's not necessary to deallocate the allocations during
> __libc_freeres anymore.

OK.
  

Patch

diff --git a/elf/dl-find_object.c b/elf/dl-find_object.c
index f81351b0ef..82f493d817 100644
--- a/elf/dl-find_object.c
+++ b/elf/dl-find_object.c
@@ -20,6 +20,7 @@ 
 #include <atomic.h>
 #include <atomic_wide_counter.h>
 #include <dl-find_object.h>
+#include <dl-protmem.h>
 #include <dlfcn.h>
 #include <ldsodefs.h>
 #include <link.h>
@@ -91,8 +92,9 @@  static struct dl_find_object_internal *_dlfo_nodelete_mappings
    to avoid data races.
 
    The memory allocations are never deallocated, but slots used for
-   objects that have been dlclose'd can be reused by dlopen.  The
-   memory can live in the regular C malloc heap.
+   objects that have been dlclose'd can be reused by dlopen.
+   Allocations come from the protected memory heap.  This makes it
+   harder to inject DWARF data.
 
    The segments are populated from the start of the list, with the
    mappings with the highest address.  Only if this segment is full,
@@ -111,9 +113,6 @@  struct dlfo_mappings_segment
      initialization; read in the TM region.  */
   struct dlfo_mappings_segment *previous;
 
-  /* Used by __libc_freeres to deallocate malloc'ed memory.  */
-  void *to_free;
-
   /* Count of array elements in use and allocated.  */
   size_t size;                  /* Read in the TM region.  */
   size_t allocated;
@@ -154,44 +153,15 @@  _dlfo_mappings_segment_count_allocated (struct dlfo_mappings_segment *seg)
 
 /* This is essentially an arbitrary value.  dlopen allocates plenty of
    memory anyway, so over-allocated a bit does not hurt.  Not having
-   many small-ish segments helps to avoid many small binary searches.
-   Not using a power of 2 means that we do not waste an extra page
-   just for the malloc header if a mapped allocation is used in the
-   glibc allocator.  */
-enum { dlfo_mappings_initial_segment_size = 63 };
-
-/* Allocate an empty segment.  This used for the first ever
-   allocation.  */
-static struct dlfo_mappings_segment *
-_dlfo_mappings_segment_allocate_unpadded (size_t size)
-{
-  if (size < dlfo_mappings_initial_segment_size)
-    size = dlfo_mappings_initial_segment_size;
-  /* No overflow checks here because the size is a mapping count, and
-     struct link_map_private is larger than what we allocate here.  */
-  enum
-    {
-      element_size = sizeof ((struct dlfo_mappings_segment) {}.objects[0])
-    };
-  size_t to_allocate = (sizeof (struct dlfo_mappings_segment)
-                        + size * element_size);
-  struct dlfo_mappings_segment *result = malloc (to_allocate);
-  if (result != NULL)
-    {
-      result->previous = NULL;
-      result->to_free = NULL; /* Minimal malloc memory cannot be freed.  */
-      result->size = 0;
-      result->allocated = size;
-    }
-  return result;
-}
+   many small-ish segments helps to avoid many small binary searches.  */
+enum { dlfo_mappings_initial_segment_size = 64 };
 
 /* Allocate an empty segment that is at least SIZE large.  PREVIOUS
    points to the chain of previously allocated segments and can be
    NULL.  */
 static struct dlfo_mappings_segment *
 _dlfo_mappings_segment_allocate (size_t size,
-                                 struct dlfo_mappings_segment * previous)
+                                 struct dlfo_mappings_segment *previous)
 {
   /* Exponential sizing policies, so that lookup approximates a binary
      search.  */
@@ -200,11 +170,10 @@  _dlfo_mappings_segment_allocate (size_t size,
     if (previous == NULL)
       minimum_growth = dlfo_mappings_initial_segment_size;
     else
-      minimum_growth = 2* previous->allocated;
+      minimum_growth = 2 * previous->allocated;
     if (size < minimum_growth)
       size = minimum_growth;
   }
-  enum { cache_line_size_estimate = 128 };
   /* No overflow checks here because the size is a mapping count, and
      struct link_map_private is larger than what we allocate here.  */
   enum
@@ -212,28 +181,13 @@  _dlfo_mappings_segment_allocate (size_t size,
       element_size = sizeof ((struct dlfo_mappings_segment) {}.objects[0])
     };
   size_t to_allocate = (sizeof (struct dlfo_mappings_segment)
-                        + size * element_size
-                        + 2 * cache_line_size_estimate);
-  char *ptr = malloc (to_allocate);
-  if (ptr == NULL)
+                        + size * element_size);
+  struct dlfo_mappings_segment *result = _dl_protmem_allocate (to_allocate);
+  if (result == NULL)
     return NULL;
-  char *original_ptr = ptr;
-  /* Start and end at a (conservative) 128-byte cache line boundary.
-     Do not use memalign for compatibility with partially interposing
-     malloc implementations.  */
-  char *end = PTR_ALIGN_DOWN (ptr + to_allocate, cache_line_size_estimate);
-  ptr = PTR_ALIGN_UP (ptr, cache_line_size_estimate);
-  struct dlfo_mappings_segment *result
-    = (struct dlfo_mappings_segment *) ptr;
   result->previous = previous;
-  result->to_free = original_ptr;
   result->size = 0;
-  /* We may have obtained slightly more space if malloc happened
-     to provide an over-aligned pointer.  */
-  result->allocated = (((uintptr_t) (end - ptr)
-                        - sizeof (struct dlfo_mappings_segment))
-                       / element_size);
-  assert (result->allocated >= size);
+  result->allocated = size;
   return result;
 }
 
@@ -577,11 +531,12 @@  _dl_find_object_init (void)
 
   /* Allocate the data structures.  */
   size_t loaded_size = _dlfo_process_initial ();
-  _dlfo_nodelete_mappings = malloc (_dlfo_nodelete_mappings_size
-                                    * sizeof (*_dlfo_nodelete_mappings));
+  _dlfo_nodelete_mappings
+    = _dl_protmem_allocate (_dlfo_nodelete_mappings_size
+                            * sizeof (*_dlfo_nodelete_mappings));
   if (loaded_size > 0)
     _dlfo_loaded_mappings[0]
-      = _dlfo_mappings_segment_allocate_unpadded (loaded_size);
+      = _dlfo_mappings_segment_allocate (loaded_size, NULL);
   if (_dlfo_nodelete_mappings == NULL
       || (loaded_size > 0 && _dlfo_loaded_mappings[0] == NULL))
     _dl_fatal_printf ("\
@@ -838,20 +793,3 @@  _dl_find_object_dlclose (struct link_map_private *map)
         return;
       }
 }
-
-void
-_dl_find_object_freeres (void)
-{
-  for (int idx = 0; idx < 2; ++idx)
-    {
-      for (struct dlfo_mappings_segment *seg = _dlfo_loaded_mappings[idx];
-           seg != NULL; )
-        {
-          struct dlfo_mappings_segment *previous = seg->previous;
-          free (seg->to_free);
-          seg = previous;
-        }
-      /* Stop searching in shared objects.  */
-      _dlfo_loaded_mappings[idx] = 0;
-    }
-}
diff --git a/elf/dl-find_object.h b/elf/dl-find_object.h
index edcc0a7755..54601e7d00 100644
--- a/elf/dl-find_object.h
+++ b/elf/dl-find_object.h
@@ -135,7 +135,4 @@  bool _dl_find_object_update (struct link_map_private *new_l) attribute_hidden;
    data structures.  Needs to be protected by loader write lock.  */
 void _dl_find_object_dlclose (struct link_map_private *l) attribute_hidden;
 
-/* Called from __libc_freeres to deallocate malloc'ed memory.  */
-void _dl_find_object_freeres (void) attribute_hidden;
-
 #endif /* _DL_FIND_OBJECT_H */
diff --git a/elf/dl-libc_freeres.c b/elf/dl-libc_freeres.c
index 88c0e444b8..066629639c 100644
--- a/elf/dl-libc_freeres.c
+++ b/elf/dl-libc_freeres.c
@@ -128,6 +128,4 @@  __rtld_libc_freeres (void)
   void *scope_free_list = GL(dl_scope_free_list);
   GL(dl_scope_free_list) = NULL;
   free (scope_free_list);
-
-  _dl_find_object_freeres ();
 }