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: [PING][PATCH][BZ #15073] Fix race in free.


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;


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