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: Arjun Shankar <arjun dot is at lostca dot se>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: DJ Delorie <dj at redhat dot com>, libc-alpha at sourceware dot org
- Date: Tue, 7 Nov 2017 12:41:34 +0000
- 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> <7fa4f3b3-53a9-9db1-2b62-eeb74609f082@redhat.com>
> >> + 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)
Yes, it is intentional. I wrote the test after Carlos suggested it on
bugzilla [1]. It made sense to me to be a bit paranoid because there was
already one test case in malloc/tst-malloc.c that did a single SIZE_MAX
sized allocation which failed to catch this regression. The new test takes
about 100ms of time on my fairly new mobile i7 x86_64.
Do you think it is worth it?
> Does this beg for a bigger comment explaining what it's doing and why?
I can post a v2 with a top level comment and a fix to a flaw that I just
noticed myself in the patch: I missed a 'free' for the block I allocate just
before testing realloc with it.
Cheers,
Arjun
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=22375#c7