This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] malloc: Remove __malloc_check_init from the API [BZ #19564]


I have rebased this patch on top of the current master.

Thanks,
Florian
malloc: Remove __malloc_check_init from the API [BZ #19564]

The stub implementation does nothing because conforming programs
cannot observe the presence of malloc checking.  It is no longer
possible to enable __malloc_check_init-style checking from
the environment.

Existing heap consistency checks are not affected by this change.

2016-05-19  Florian Weimer  <fweimer@redhat.com>

	[BZ #19564]
	Remove __malloc_check_init from the API.
	* malloc/malloc.h (__malloc_check_init): Remove.
	* malloc/arena.c (ptmalloc_init): Do not call __malloc_check_init.
	* malloc/hooks.c (__malloc_check_init): Turn into a NOP and compat
	symbol.
	(using_malloc_checking, disallow_malloc_check, magicbyte)
	(malloc_check_get_size, mem2mem_check, mem2chunk_check, top_check)
	(malloc_check, free_check, realloc_check, memalign_check): Remove.
	(__malloc_set_state): Do not disable malloc checking because it is
	always disabled.
	* malloc/malloc.c (mem2mem_check, top_check, malloc_check)
	(free_check, realloc_check, memalign_check): Remove declarations.
	(musable): Do not call malloc_check_get_size.
	* malloc/tst-malloc-usable.c (do_test): Do not assume that
	malloc_usable_size returns the exact size.

diff --git a/malloc/arena.c b/malloc/arena.c
index 1dd9dee..32bff63 100644
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -335,11 +335,7 @@ ptmalloc_init (void)
         }
     }
   if (s && s[0])
-    {
-      __libc_mallopt (M_CHECK_ACTION, (int) (s[0] - '0'));
-      if (check_action != 0)
-        __malloc_check_init ();
-    }
+    __libc_mallopt (M_CHECK_ACTION, (int) (s[0] - '0'));
   void (*hook) (void) = atomic_forced_read (__malloc_initialize_hook);
   if (hook != NULL)
     (*hook)();
diff --git a/malloc/hooks.c b/malloc/hooks.c
index 45241f2..555b37e 100644
--- a/malloc/hooks.c
+++ b/malloc/hooks.c
@@ -49,404 +49,16 @@ memalign_hook_ini (size_t alignment, size_t sz, const void *caller)
   return __libc_memalign (alignment, sz);
 }
 
-/* Whether we are using malloc checking.  */
-static int using_malloc_checking;
-
-/* A flag that is set by malloc_set_state, to signal that malloc checking
-   must not be enabled on the request from the user (via the MALLOC_CHECK_
-   environment variable).  It is reset by __malloc_check_init to tell
-   malloc_set_state that the user has requested malloc checking.
-
-   The purpose of this flag is to make sure that malloc checking is not
-   enabled when the heap to be restored was constructed without malloc
-   checking, and thus does not contain the required magic bytes.
-   Otherwise the heap would be corrupted by calls to free and realloc.  If
-   it turns out that the heap was created with malloc checking and the
-   user has requested it malloc_set_state just calls __malloc_check_init
-   again to enable it.  On the other hand, reusing such a heap without
-   further malloc checking is safe.  */
-static int disallow_malloc_check;
-
-/* Activate a standard set of debugging hooks. */
+/* This used to activate debugging hooks.  Debugging hooks are
+   unobservable in a correct program, so the current implementation
+   does nothing, but is still provided for backwards
+   compatibility.  */
+#if SHLIB_COMPAT (libc, GLIBC_2_0, GLIBC_2_24)
 void
 __malloc_check_init (void)
 {
-  if (disallow_malloc_check)
-    {
-      disallow_malloc_check = 0;
-      return;
-    }
-  using_malloc_checking = 1;
-  __malloc_hook = malloc_check;
-  __free_hook = free_check;
-  __realloc_hook = realloc_check;
-  __memalign_hook = memalign_check;
-}
-
-/* A simple, standard set of debugging hooks.  Overhead is `only' one
-   byte per chunk; still this will catch most cases of double frees or
-   overruns.  The goal here is to avoid obscure crashes due to invalid
-   usage, unlike in the MALLOC_DEBUG code. */
-
-static unsigned char
-magicbyte (const void *p)
-{
-  unsigned char magic;
-
-  magic = (((uintptr_t) p >> 3) ^ ((uintptr_t) p >> 11)) & 0xFF;
-  /* Do not return 1.  See the comment in mem2mem_check().  */
-  if (magic == 1)
-    ++magic;
-  return magic;
-}
-
-
-/* Visualize the chunk as being partitioned into blocks of 255 bytes from the
-   highest address of the chunk, downwards.  The end of each block tells
-   us the size of that block, up to the actual size of the requested
-   memory.  Our magic byte is right at the end of the requested size, so we
-   must reach it with this iteration, otherwise we have witnessed a memory
-   corruption.  */
-static size_t
-malloc_check_get_size (mchunkptr p)
-{
-  size_t size;
-  unsigned char c;
-  unsigned char magic = magicbyte (p);
-
-  assert (using_malloc_checking == 1);
-
-  for (size = chunksize (p) - 1 + (chunk_is_mmapped (p) ? 0 : SIZE_SZ);
-       (c = ((unsigned char *) p)[size]) != magic;
-       size -= c)
-    {
-      if (c <= 0 || size < (c + 2 * SIZE_SZ))
-        {
-          malloc_printerr (check_action, "malloc_check_get_size: memory corruption",
-                           chunk2mem (p),
-			   chunk_is_mmapped (p) ? NULL : arena_for_chunk (p));
-          return 0;
-        }
-    }
-
-  /* chunk2mem size.  */
-  return size - 2 * SIZE_SZ;
-}
-
-/* Instrument a chunk with overrun detector byte(s) and convert it
-   into a user pointer with requested size req_sz. */
-
-static void *
-internal_function
-mem2mem_check (void *ptr, size_t req_sz)
-{
-  mchunkptr p;
-  unsigned char *m_ptr = ptr;
-  size_t max_sz, block_sz, i;
-  unsigned char magic;
-
-  if (!ptr)
-    return ptr;
-
-  p = mem2chunk (ptr);
-  magic = magicbyte (p);
-  max_sz = chunksize (p) - 2 * SIZE_SZ;
-  if (!chunk_is_mmapped (p))
-    max_sz += SIZE_SZ;
-  for (i = max_sz - 1; i > req_sz; i -= block_sz)
-    {
-      block_sz = MIN (i - req_sz, 0xff);
-      /* Don't allow the magic byte to appear in the chain of length bytes.
-         For the following to work, magicbyte cannot return 0x01.  */
-      if (block_sz == magic)
-        --block_sz;
-
-      m_ptr[i] = block_sz;
-    }
-  m_ptr[req_sz] = magic;
-  return (void *) m_ptr;
-}
-
-/* Convert a pointer to be free()d or realloc()ed to a valid chunk
-   pointer.  If the provided pointer is not valid, return NULL. */
-
-static mchunkptr
-internal_function
-mem2chunk_check (void *mem, unsigned char **magic_p)
-{
-  mchunkptr p;
-  INTERNAL_SIZE_T sz, c;
-  unsigned char magic;
-
-  if (!aligned_OK (mem))
-    return NULL;
-
-  p = mem2chunk (mem);
-  sz = chunksize (p);
-  magic = magicbyte (p);
-  if (!chunk_is_mmapped (p))
-    {
-      /* Must be a chunk in conventional heap memory. */
-      int contig = contiguous (&main_arena);
-      if ((contig &&
-           ((char *) p < mp_.sbrk_base ||
-            ((char *) p + sz) >= (mp_.sbrk_base + main_arena.system_mem))) ||
-          sz < MINSIZE || sz & MALLOC_ALIGN_MASK || !inuse (p) ||
-          (!prev_inuse (p) && (p->prev_size & MALLOC_ALIGN_MASK ||
-                               (contig && (char *) prev_chunk (p) < mp_.sbrk_base) ||
-                               next_chunk (prev_chunk (p)) != p)))
-        return NULL;
-
-      for (sz += SIZE_SZ - 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c)
-        {
-          if (c == 0 || sz < (c + 2 * SIZE_SZ))
-            return NULL;
-        }
-    }
-  else
-    {
-      unsigned long offset, page_mask = GLRO (dl_pagesize) - 1;
-
-      /* mmap()ed chunks have MALLOC_ALIGNMENT or higher power-of-two
-         alignment relative to the beginning of a page.  Check this
-         first. */
-      offset = (unsigned long) mem & page_mask;
-      if ((offset != MALLOC_ALIGNMENT && offset != 0 && offset != 0x10 &&
-           offset != 0x20 && offset != 0x40 && offset != 0x80 && offset != 0x100 &&
-           offset != 0x200 && offset != 0x400 && offset != 0x800 && offset != 0x1000 &&
-           offset < 0x2000) ||
-          !chunk_is_mmapped (p) || (p->size & PREV_INUSE) ||
-          ((((unsigned long) p - p->prev_size) & page_mask) != 0) ||
-          ((p->prev_size + sz) & page_mask) != 0)
-        return NULL;
-
-      for (sz -= 1; (c = ((unsigned char *) p)[sz]) != magic; sz -= c)
-        {
-          if (c == 0 || sz < (c + 2 * SIZE_SZ))
-            return NULL;
-        }
-    }
-  ((unsigned char *) p)[sz] ^= 0xFF;
-  if (magic_p)
-    *magic_p = (unsigned char *) p + sz;
-  return p;
-}
-
-/* Check for corruption of the top chunk, and try to recover if
-   necessary. */
-
-static int
-internal_function
-top_check (void)
-{
-  mchunkptr t = top (&main_arena);
-  char *brk, *new_brk;
-  INTERNAL_SIZE_T front_misalign, sbrk_size;
-  unsigned long pagesz = GLRO (dl_pagesize);
-
-  if (t == initial_top (&main_arena) ||
-      (!chunk_is_mmapped (t) &&
-       chunksize (t) >= MINSIZE &&
-       prev_inuse (t) &&
-       (!contiguous (&main_arena) ||
-        (char *) t + chunksize (t) == mp_.sbrk_base + main_arena.system_mem)))
-    return 0;
-
-  malloc_printerr (check_action, "malloc: top chunk is corrupt", t,
-		   &main_arena);
-
-  /* Try to set up a new top chunk. */
-  brk = MORECORE (0);
-  front_misalign = (unsigned long) chunk2mem (brk) & MALLOC_ALIGN_MASK;
-  if (front_misalign > 0)
-    front_misalign = MALLOC_ALIGNMENT - front_misalign;
-  sbrk_size = front_misalign + mp_.top_pad + MINSIZE;
-  sbrk_size += pagesz - ((unsigned long) (brk + sbrk_size) & (pagesz - 1));
-  new_brk = (char *) (MORECORE (sbrk_size));
-  if (new_brk == (char *) (MORECORE_FAILURE))
-    {
-      __set_errno (ENOMEM);
-      return -1;
-    }
-  /* Call the `morecore' hook if necessary.  */
-  void (*hook) (void) = atomic_forced_read (__after_morecore_hook);
-  if (hook)
-    (*hook)();
-  main_arena.system_mem = (new_brk - mp_.sbrk_base) + sbrk_size;
-
-  top (&main_arena) = (mchunkptr) (brk + front_misalign);
-  set_head (top (&main_arena), (sbrk_size - front_misalign) | PREV_INUSE);
-
-  return 0;
-}
-
-static void *
-malloc_check (size_t sz, const void *caller)
-{
-  void *victim;
-
-  if (sz + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
-
-  (void) mutex_lock (&main_arena.mutex);
-  victim = (top_check () >= 0) ? _int_malloc (&main_arena, sz + 1) : NULL;
-  (void) mutex_unlock (&main_arena.mutex);
-  return mem2mem_check (victim, sz);
 }
-
-static void
-free_check (void *mem, const void *caller)
-{
-  mchunkptr p;
-
-  if (!mem)
-    return;
-
-  (void) mutex_lock (&main_arena.mutex);
-  p = mem2chunk_check (mem, NULL);
-  if (!p)
-    {
-      (void) mutex_unlock (&main_arena.mutex);
-
-      malloc_printerr (check_action, "free(): invalid pointer", mem,
-		       &main_arena);
-      return;
-    }
-  if (chunk_is_mmapped (p))
-    {
-      (void) mutex_unlock (&main_arena.mutex);
-      munmap_chunk (p);
-      return;
-    }
-  _int_free (&main_arena, p, 1);
-  (void) mutex_unlock (&main_arena.mutex);
-}
-
-static void *
-realloc_check (void *oldmem, size_t bytes, const void *caller)
-{
-  INTERNAL_SIZE_T nb;
-  void *newmem = 0;
-  unsigned char *magic_p;
-
-  if (bytes + 1 == 0)
-    {
-      __set_errno (ENOMEM);
-      return NULL;
-    }
-  if (oldmem == 0)
-    return malloc_check (bytes, NULL);
-
-  if (bytes == 0)
-    {
-      free_check (oldmem, NULL);
-      return NULL;
-    }
-  (void) mutex_lock (&main_arena.mutex);
-  const mchunkptr oldp = mem2chunk_check (oldmem, &magic_p);
-  (void) mutex_unlock (&main_arena.mutex);
-  if (!oldp)
-    {
-      malloc_printerr (check_action, "realloc(): invalid pointer", oldmem,
-		       &main_arena);
-      return malloc_check (bytes, NULL);
-    }
-  const INTERNAL_SIZE_T oldsize = chunksize (oldp);
-
-  checked_request2size (bytes + 1, nb);
-  (void) mutex_lock (&main_arena.mutex);
-
-  if (chunk_is_mmapped (oldp))
-    {
-#if HAVE_MREMAP
-      mchunkptr newp = mremap_chunk (oldp, nb);
-      if (newp)
-        newmem = chunk2mem (newp);
-      else
 #endif
-      {
-        /* Note the extra SIZE_SZ overhead. */
-        if (oldsize - SIZE_SZ >= nb)
-          newmem = oldmem; /* do nothing */
-        else
-          {
-            /* Must alloc, copy, free. */
-            if (top_check () >= 0)
-              newmem = _int_malloc (&main_arena, bytes + 1);
-            if (newmem)
-              {
-                memcpy (newmem, oldmem, oldsize - 2 * SIZE_SZ);
-                munmap_chunk (oldp);
-              }
-          }
-      }
-    }
-  else
-    {
-      if (top_check () >= 0)
-        {
-          INTERNAL_SIZE_T nb;
-          checked_request2size (bytes + 1, nb);
-          newmem = _int_realloc (&main_arena, oldp, oldsize, nb);
-        }
-    }
-
-  /* mem2chunk_check changed the magic byte in the old chunk.
-     If newmem is NULL, then the old chunk will still be used though,
-     so we need to invert that change here.  */
-  if (newmem == NULL)
-    *magic_p ^= 0xFF;
-
-  (void) mutex_unlock (&main_arena.mutex);
-
-  return mem2mem_check (newmem, bytes);
-}
-
-static void *
-memalign_check (size_t alignment, size_t bytes, const void *caller)
-{
-  void *mem;
-
-  if (alignment <= MALLOC_ALIGNMENT)
-    return malloc_check (bytes, NULL);
-
-  if (alignment < MINSIZE)
-    alignment = MINSIZE;
-
-  /* If the alignment is greater than SIZE_MAX / 2 + 1 it cannot be a
-     power of 2 and will cause overflow in the check below.  */
-  if (alignment > SIZE_MAX / 2 + 1)
-    {
-      __set_errno (EINVAL);
-      return 0;
-    }
-
-  /* Check for overflow.  */
-  if (bytes > SIZE_MAX - alignment - MINSIZE)
-    {
-      __set_errno (ENOMEM);
-      return 0;
-    }
-
-  /* Make sure alignment is power of 2.  */
-  if (!powerof2 (alignment))
-    {
-      size_t a = MALLOC_ALIGNMENT * 2;
-      while (a < alignment)
-        a <<= 1;
-      alignment = a;
-    }
-
-  (void) mutex_lock (&main_arena.mutex);
-  mem = (top_check () >= 0) ? _int_memalign (&main_arena, alignment, bytes + 1) :
-        NULL;
-  (void) mutex_unlock (&main_arena.mutex);
-  return mem2mem_check (mem, bytes);
-}
-
 
 /* Get/set state: malloc_get_state() records the current state of all
    malloc variables (_except_ for the actual heap contents and `hook'
@@ -535,7 +147,7 @@ __malloc_get_state (void)
   ms->max_n_mmaps = mp_.max_n_mmaps;
   ms->mmapped_mem = mp_.mmapped_mem;
   ms->max_mmapped_mem = mp_.max_mmapped_mem;
-  ms->using_malloc_checking = using_malloc_checking;
+  ms->using_malloc_checking = 0;
   ms->max_fast = get_max_fast ();
   ms->arena_test = mp_.arena_test;
   ms->arena_max = mp_.arena_max;
@@ -562,12 +174,11 @@ __malloc_set_state (void *msptr)
      calls calloc and thus must be called only afterwards, so there
      cannot be more than one thread when we reach this point.  */
 
-  /* Disable the malloc hooks (and malloc checking).  */
+  /* Disable the malloc hooks.  */
   __malloc_hook = NULL;
   __realloc_hook = NULL;
   __free_hook = NULL;
   __memalign_hook = NULL;
-  using_malloc_checking = 0;
 
   /* Patch the dumped heap.  We no longer try to integrate into the
      existing heap.  Instead, we mark the existing chunks as mmapped.
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 44524ff..41126f9 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1061,20 +1061,11 @@ static void*  _mid_memalign(size_t, size_t, void *);
 
 static void malloc_printerr(int action, const char *str, void *ptr, mstate av);
 
-static void* internal_function mem2mem_check(void *p, size_t sz);
-static int internal_function top_check(void);
 static void internal_function munmap_chunk(mchunkptr p);
 #if HAVE_MREMAP
 static mchunkptr internal_function mremap_chunk(mchunkptr p, size_t new_size);
 #endif
 
-static void*   malloc_check(size_t sz, const void *caller);
-static void      free_check(void* mem, const void *caller);
-static void*   realloc_check(void* oldmem, size_t bytes,
-			       const void *caller);
-static void*   memalign_check(size_t alignment, size_t bytes,
-				const void *caller);
-
 /* ------------------ MMAP support ------------------  */
 
 
@@ -4612,9 +4603,6 @@ musable (void *mem)
     {
       p = mem2chunk (mem);
 
-      if (__builtin_expect (using_malloc_checking == 1, 0))
-        return malloc_check_get_size (p);
-
       if (chunk_is_mmapped (p))
         return chunksize (p) - 2 * SIZE_SZ;
       else if (inuse (p))
diff --git a/malloc/malloc.h b/malloc/malloc.h
index d95a315..2d513c0 100644
--- a/malloc/malloc.h
+++ b/malloc/malloc.h
@@ -163,9 +163,5 @@ extern void *(*__MALLOC_HOOK_VOLATILE __memalign_hook)(size_t __alignment,
 __MALLOC_DEPRECATED;
 extern void (*__MALLOC_HOOK_VOLATILE __after_morecore_hook) (void);
 
-/* Activate a standard set of debugging hooks. */
-extern void __malloc_check_init (void) __THROW __MALLOC_DEPRECATED;
-
-
 __END_DECLS
 #endif /* malloc.h */
diff --git a/malloc/tst-malloc-usable.c b/malloc/tst-malloc-usable.c
index ca67ed7..dc7d4b8 100644
--- a/malloc/tst-malloc-usable.c
+++ b/malloc/tst-malloc-usable.c
@@ -1,4 +1,4 @@
-/* Ensure that malloc_usable_size returns the request size with
+/* Ensure that malloc_usable_size returns a valid size with
    MALLOC_CHECK_ exported to a positive value.
 
    Copyright (C) 2012-2016 Free Software Foundation, Inc.
@@ -25,23 +25,35 @@
 static int
 do_test (void)
 {
-  size_t usable_size;
-  void *p = malloc (7);
-  if (!p)
-    {
-      printf ("memory allocation failed\n");
-      return 1;
-    }
+  size_t old_usable_size = 0;
 
-  usable_size = malloc_usable_size (p);
-  if (usable_size != 7)
+  /* Test with various padding byte values so that it is very likely
+     we trigger any kind of consistency check if the return value from
+     malloc_usable_size is incorrect.  */
+  for (int pad = 0; pad <= 255; ++pad)
     {
-      printf ("malloc_usable_size: expected 7 but got %zu\n", usable_size);
-      return 1;
+      void *p = malloc (7);
+      if (!p)
+        {
+          printf ("memory allocation failed\n");
+          return 1;
+        }
+      size_t usable_size = malloc_usable_size (p);
+      if (usable_size < 7)
+        {
+          printf ("malloc_usable_size: expected at least 7 but got %zu\n",
+                  usable_size);
+          return 1;
+        }
+      if (usable_size != old_usable_size)
+        {
+          printf ("info: malloc_usable_size: %zu\n", usable_size);
+          old_usable_size = usable_size;
+        }
+      memset (p, pad, usable_size);
+      free (p);
     }
 
-  memset (p, 0, usable_size);
-  free (p);
   return 0;
 }
 

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]