This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Fix assertion in malloc.c:tcache_get


* Adam Maris:

> On Mon, Feb 11, 2019 at 3:39 PM Florian Weimer <fweimer@redhat.com> wrote:
>>
>> * Adam Maris:
>>
>> > On Wed, Feb 6, 2019 at 5:37 PM DJ Delorie <dj@delorie.com> wrote:
>> >>
>> >>
>> >> "Carlos O'Donell" <carlos@redhat.com> writes:
>> >> > On 2/4/19 6:36 PM, DJ Delorie wrote:
>> >> >> Joseph Myers <joseph@codesourcery.com> writes:
>> >> >>> -  assert (tcache->entries[tc_idx] > 0);
>> >> >>> +  assert (tcache->counts[tc_idx] > 0);
>> >> >>
>> >> >> Yes please :-)
>> >> >
>> >> > Did we backport this anywhere that needs this fix?
>> >>
>> >> Amusingly, the code still kinda works correctly as long as pointers are
>> >> considered unsigned.  The assert will only trigger if the pointer is
>> >> NULL, which would have caused a segfault a few lines later anyway.
>> >
>> > Not really, it was a bug that could be exploited if there was a memory
>> > corruption, see:
>> >
>> > https://sourceware.org/bugzilla/show_bug.cgi?id=23733
>> > https://sourceware.org/ml/libc-alpha/2018-11/msg00323.html
>> >
>> > I guess the patch fell through the cracks, I should have pinged you
>> > about it earlier.
>>
>> In this case, it should be an anssert, but a regular check with a call
>> to malloc_printerr on failure.
>>
>
> FWIW, in my first unpublished version I wanted to also add another
> check in __libc_malloc() before calling tcache_get(), but DJ said
> assert is fine since we don't call NDEBUG anywhere.

We support building with NDEBUG.  I'm not sure if any of the major
distributions still do it.  Fedora stopped doing so a few years ago.

So I think the assert should be changed because it is a security
hardening check and not strictly an internal implementation detail.

Thanks,
Florian


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]