This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PING][PATCH][BZ #15073] Fix race in free.
- From: Maxim Kuvyrkov <maxim at kugelworks dot com>
- To: OndÅej BÃlka <neleai at seznam dot cz>
- Cc: GLIBC Devel <libc-alpha at sourceware dot org>
- Date: Fri, 20 Dec 2013 11:49:07 +1300
- Subject: Re: [PING][PATCH][BZ #15073] Fix race in free.
- Authentication-results: sourceware.org; auth=none
- References: <20131210194349 dot GA19644 at domone dot podge> <20131218102757 dot GC6559 at domone dot podge>
Hi Ondrej,
I'm reviewing your patch (and trying to make sense of _int_free code). Will report back in couple of days.
Thanks,
--
Maxim Kuvyrkov
www.kugelworks.com
On 18/12/2013, at 11:27 pm, OndÅej BÃlka <neleai@seznam.cz> wrote:
> ping
> On Tue, Dec 10, 2013 at 08:43:49PM +0100, OndÅej BÃlka wrote:
>> Hi,
>>
>> While actual implementation of atomic returning to fastbin is correct
>> this could not be sayed about 'sanity' check surrounding it.
>>
>> When we read fastbin entry there is no quarantee that memory will be
>> deallocated in meantime.
>>
>> Yet we try to dereference that pointer for a check that does not make
>> any sense. We check if old chunk belongs to fastbin we are adding
>> element. If old gets consolidated to bigger chunk we get a spurious
>> memory corruption error, while if we continue a subsequent compare and
>> swap will fail.
>>
>> Solution is simply drop that check.
>>
>> [BZ #15073]
>> * malloc/malloc.c (_int_free): Remove sanity check that causes
>> race condition.
>>
>> diff --git a/malloc/malloc.c b/malloc/malloc.c
>> index b1668b5..a05d4bb 100644
>> --- a/malloc/malloc.c
>> +++ b/malloc/malloc.c
>> @@ -3785,7 +3785,6 @@ _int_free(mstate av, mchunkptr p, int have_lock)
>>
>> mchunkptr fd;
>> mchunkptr old = *fb;
>> - unsigned int old_idx = ~0u;
>> do
>> {
>> /* Another simple check: make sure the top of the bin is not the
>> @@ -3795,13 +3794,10 @@ _int_free(mstate av, mchunkptr p, int have_lock)
>> errstr = "double free or corruption (fasttop)";
>> goto errout;
>> }
>> - if (old != NULL)
>> - old_idx = fastbin_index(chunksize(old));
>> p->fd = fd = old;
>> }
>> while ((old = catomic_compare_and_exchange_val_rel (fb, p, fd)) != fd);
>>
>> - if (fd != NULL && __builtin_expect (old_idx != idx, 0))
>> + if (fd != NULL)
>> {
>> errstr = "invalid fastbin entry (free)";
>> goto errout;