Re: [PATCH] malloc/hooks.c: Correct check for overflow in memalign_check.

[Will Newton <will dot newton at linaro dot org>]

On 9 October 2013 19:36, Carlos O'Donell <carlos@redhat.com> wrote:

Hi Carlos,

> 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.

I took that as an OK and applied the patch.

-- 
Will Newton
Toolchain Working Group, Linaro