This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Fix integer overflow in malloc when tcache is enabled [BZ #22375]
- From: Carlos O'Donell <carlos at redhat dot com>
- To: DJ Delorie <dj at redhat dot com>, Arjun Shankar <arjun dot is at lostca dot se>
- Cc: libc-alpha at sourceware dot org
- Date: Mon, 6 Nov 2017 09:54:53 -0800
- Subject: Re: [PATCH] Fix integer overflow in malloc when tcache is enabled [BZ #22375]
- Authentication-results: sourceware.org; auth=none
- References: <xnfu9r1eir.fsf@greed.delorie.com>
On 11/06/2017 09:51 AM, DJ Delorie wrote:
> Arjun Shankar <arjun.is@lostca.se> writes:
>> When the per-thread cache is enabled, __libc_malloc uses request2size (which
>> does not perform an overflow check) to calculate the chunk size from the
>> requested allocation size. This leads to an integer overflow causing malloc
>> to incorrectly return the last successfully allocated block when called with
>> a very large size argument (close to SIZE_MAX) instead of returning NULL and
>> setting errno to ENOMEM.
>
>> [BZ #22375]
>> * malloc/malloc.c (__libc_malloc): Use checked_request2size instead
>> of request2size.
>> * malloc/tst-malloc-too-large.c: New test.
>> * malloc/Makefile: Add tst-malloc-too-large.
>
>> + for (lsbs = FIFTY_ON_BITS; lsbs > FIFTY_ON_BITS - (1UL << 14); lsbs--)
>> + for (msbs = FOURTEEN_ON_BITS; msbs >= 1; msbs--)
>
> Is it intentional that you're doing 32,768 allocations when this runs?
> While this shouldn't take time on modern 64-bit hardware, I wonder if
> this level of overkill is justified (not that I'm ever against overkill
> *in general* but these tests get run a lot)
Does this beg for a bigger comment explaining what it's doing and why?
--
Cheers,
Carlos.