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

["Carlos O'Donell" <carlos at redhat dot com>]

On 10/10/2013 09:56 AM, Will Newton wrote:
> 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.

It was. Sorry for not being sufficiently explicit.

Cheers,
Carlos.