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/hooks.c: Correct check for overflow in memalign_check.


On 10/09/2013 09:46 AM, Will Newton wrote:
> 
> A large value of bytes passed to memalign_check can cause an integer
> overflow in _int_memalign and heap corruption. This issue can be
> exposed by running tst-memalign with MALLOC_CHECK_=3.
> 
> ChangeLog:
> 
> 2013-10-09  Will Newton  <will.newton@linaro.org>
> 
> 	* malloc/hooks.c (memalign_check): Ensure the value of bytes
> 	passed to _int_memalign does not overflow.
> ---
>  malloc/hooks.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/malloc/hooks.c b/malloc/hooks.c
> index 8c25846..3f663bb 100644
> --- a/malloc/hooks.c
> +++ b/malloc/hooks.c
> @@ -361,10 +361,13 @@ memalign_check(size_t alignment, size_t bytes, const void *caller)
>    if (alignment <= MALLOC_ALIGNMENT) return malloc_check(bytes, NULL);
>    if (alignment <  MINSIZE) alignment = MINSIZE;
> 
> -  if (bytes+1 == 0) {
> -    __set_errno (ENOMEM);
> -    return NULL;
> -  }
> +  /* Check for overflow.  */
> +  if (bytes > SIZE_MAX - alignment - MINSIZE)
> +    {
> +      __set_errno (ENOMEM);
> +      return 0;
> +    }
> +
>    (void)mutex_lock(&main_arena.mutex);
>    mem = (top_check() >= 0) ? _int_memalign(&main_arena, alignment, bytes+1) :
>      NULL;
> 

This is better than `bytes+1' so it should go in immediately to fix
the test regression under _MALLOC_CHECK=3.

We still need to deal with `alignment' having no upper bound.

Cheers,
Carlos.


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