This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] malloc/hooks.c: Correct check for overflow in memalign_check.
- From: "Carlos O'Donell" <carlos at redhat dot com>
- To: Will Newton <will dot newton at linaro dot org>
- Cc: libc-alpha at sourceware dot org, patches at linaro dot org
- Date: Wed, 09 Oct 2013 14:36:38 -0400
- Subject: Re: [PATCH] malloc/hooks.c: Correct check for overflow in memalign_check.
- Authentication-results: sourceware.org; auth=none
- References: <52555E49 dot 4050506 at linaro dot org>
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.