[prev in list] [next in list] [prev in thread] [next in thread] 

List:       ipsec-tools-devel
Subject:    Re: [Ipsec-tools-devel] [PATCH] potential crash in racoon
From:       Timo_Teräs <timo.teras () iki ! fi>
Date:       2008-06-11 6:24:01
Message-ID: 484F6F81.6030401 () iki ! fi
[Download RAW message or body]

Timo Teräs wrote:
> Atis Elsts wrote:
>> I noticed that phase1 handler can be freed one time too many.
>> The problem was introduced during memory leak fixes.
>>
>> Attached path fixes that, as well as removes some unnecessary (to my
>> understanding) remph1() calls.
> 
> The remph1() calls are not necessary if insph1() has not been called yet.
> Otherwise it is needed. Actually, calling remph1() if insph1() has not been
> called can crash. So the removal of those is correct in appropriate places.
> 
>> @@ -1072,11 +1071,8 @@
>>  	iph1->approval = NULL;
>>  
>>  	/* XXX copy remote address */
>> -	if (copy_ph1addresses(iph1, rmconf, remote, local) < 0) {
>> -		remph1(iph1);
>> -		delph1(iph1);
>> +	if (copy_ph1addresses(iph1, rmconf, remote, local) < 0)
>>  		return NULL;
>> -	}
>>  
>>  	(void)insph1(iph1);
> 
> Removal of "delph1" here isn't right. Removal of remph1() is ok.

Rereading the patch again. And sorry, my mistake, I didn't read
copy_ph1addresses().

I just looked at the flow on the modified functions. It appears that
copy_ph1addresses() calls delph1() too. IMHO, that is bad and
copy_ph1addresses() should be fixed to not call delph1() instead of the
otherway around. It makes reading the calling functions easier and it is
hard to remember which functions did the delph1() and which didn't.

Cheers,
  Timo

-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://sourceforge.net/services/buy/index.php
_______________________________________________
Ipsec-tools-devel mailing list
Ipsec-tools-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ipsec-tools-devel

[prev in list] [next in list] [prev in thread] [next in thread]