Patchwork [v2,4/7] malloc: Ensure lower bound on chunk size in __libc_realloc.

login
register
mail settings
Submitter Istvan Kurucsai
Date Nov. 7, 2017, 3:27 p.m.
Message ID <1510068430-27816-5-git-send-email-pistukem@gmail.com>
Download mbox | patch
Permalink /patch/24142/
State New
Headers show

Comments

Istvan Kurucsai - Nov. 7, 2017, 3:27 p.m.
Under some circumstances, a chunk size of SIZE_SZ could lead to an underflow
when calculating the length argument of memcpy.

   * malloc/malloc.c (__libc_realloc): Check chunk size.
---
 malloc/malloc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
Florian Weimer - Aug. 17, 2018, 2:12 p.m.
On 11/07/2017 04:27 PM, Istvan Kurucsai wrote:
> Under some circumstances, a chunk size of SIZE_SZ could lead to an underflow
> when calculating the length argument of memcpy.
> 
>     * malloc/malloc.c (__libc_realloc): Check chunk size.
> ---
>   malloc/malloc.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 51d703c..8e48952 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -3154,8 +3154,9 @@ __libc_realloc (void *oldmem, size_t bytes)
>        accident or by "design" from some intruder.  We need to bypass
>        this check for dumped fake mmap chunks from the old main arena
>        because the new malloc may provide additional alignment.  */
> -  if ((__builtin_expect ((uintptr_t) oldp > (uintptr_t) -oldsize, 0)
> -       || __builtin_expect (misaligned_chunk (oldp), 0))
> +  if ((__glibc_unlikely ((uintptr_t) oldp > (uintptr_t) -oldsize)
> +       || __glibc_unlikely (misaligned_chunk (oldp))
> +       || __glibc_unlikely (oldsize <= 2 * SIZE_SZ))
>         && !DUMPED_MAIN_ARENA_CHUNK (oldp))
>         malloc_printerr ("realloc(): invalid pointer");

DJ, can you benchmark this change?  If it turns out this is visible in 
profiles, we may have to move this check closer to the actual memcpy.

Thanks,
Florian

Patch

diff --git a/malloc/malloc.c b/malloc/malloc.c
index 51d703c..8e48952 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3154,8 +3154,9 @@  __libc_realloc (void *oldmem, size_t bytes)
      accident or by "design" from some intruder.  We need to bypass
      this check for dumped fake mmap chunks from the old main arena
      because the new malloc may provide additional alignment.  */
-  if ((__builtin_expect ((uintptr_t) oldp > (uintptr_t) -oldsize, 0)
-       || __builtin_expect (misaligned_chunk (oldp), 0))
+  if ((__glibc_unlikely ((uintptr_t) oldp > (uintptr_t) -oldsize)
+       || __glibc_unlikely (misaligned_chunk (oldp))
+       || __glibc_unlikely (oldsize <= 2 * SIZE_SZ))
       && !DUMPED_MAIN_ARENA_CHUNK (oldp))
       malloc_printerr ("realloc(): invalid pointer");