This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] malloc: Remove __malloc_check_init from the API [BZ #19564]
- From: fweimer at redhat dot com (Florian Weimer)
- To: libc-alpha at sourceware dot org
- Date: Wed, 11 May 2016 20:49:28 +0200
- Subject: [PATCH] malloc: Remove __malloc_check_init from the API [BZ #19564]
- Authentication-results: sourceware.org; auth=none
2016-05-10 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): Fail if the dumped heap requires malloc
checking.
* 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 0d2bb96..3e04fa2 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;
@@ -551,13 +163,14 @@ __malloc_set_state (void *msptr)
size_t i;
mbinptr b;
- disallow_malloc_check = 1;
ptmalloc_init ();
if (ms->magic != MALLOC_STATE_MAGIC)
return -1;
- /* Must fail if the major version is too high. */
- if ((ms->version & ~0xffl) > (MALLOC_STATE_VERSION & ~0xffl))
+ /* Must fail if the major version is too high. We no longer
+ implement malloc checking and cannot restore those heaps. */
+ if ((ms->version & ~0xffl) > (MALLOC_STATE_VERSION & ~0xffl)
+ || ms->using_malloc_checking)
return -2;
(void) mutex_lock (&main_arena.mutex);
@@ -635,22 +248,6 @@ __malloc_set_state (void *msptr)
mp_.mmapped_mem = ms->mmapped_mem;
mp_.max_mmapped_mem = ms->max_mmapped_mem;
/* add version-dependent code here */
- if (ms->version >= 1)
- {
- /* Check whether it is safe to enable malloc checking, or whether
- it is necessary to disable it. */
- if (ms->using_malloc_checking && !using_malloc_checking &&
- !disallow_malloc_check)
- __malloc_check_init ();
- else if (!ms->using_malloc_checking && using_malloc_checking)
- {
- __malloc_hook = NULL;
- __free_hook = NULL;
- __realloc_hook = NULL;
- __memalign_hook = NULL;
- using_malloc_checking = 0;
- }
- }
if (ms->version >= 4)
{
mp_.arena_test = ms->arena_test;
diff --git a/malloc/malloc.c b/malloc/malloc.c
index ea97df2..eff7e18 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 ------------------ */
@@ -4575,9 +4566,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;
}