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

List:       ipsec-tools-devel
Subject:    Re: [Ipsec-tools-devel] memory leak during rsasig verification
From:       Timo_Teräs <timo.teras () iki ! fi>
Date:       2011-02-18 5:53:20
Message-ID: 4D5E0950.7020005 () iki ! fi
[Download RAW message or body]

On 02/18/2011 03:02 AM, Roman Hoog Antink wrote:
> On 17/02/11 23:44, Timo Teräs wrote:
>> Other parts of code assign also rsa_candidate to NULL after freeing it.
>> But IMHO, we should free it in delph1() since it's a structure member
>> and that function is supposed to cleanly nuke the structure.
> 
> Thank's for the hint about resetting the pointer just after the free().
> 
> However, freeing rsa_candidates during iph1 deletion is too late.
> get_plainrsa_fromlocal() uses the same variable (namely rsa_candidates)
> for both purposes: once for the local key and once for the remote key.
> But in case of the local key, the variable gets overwritten.
> 
> A breakpoint at oakley.c:1675 with condition (iph1->rsa_candidates !=
> NULL) is being hit for every tunnel shortly after launching racoon. And
> also when removing a tunnel from config with a subsequent reload. These
> occurrences obviously happen long before that iph1 will be deleted.
> 
> The breakpoint shows that rsa_candidates is used with my=0 first and
> later rsa_candidates is used again from the same iph1, overwriting the
> pointer and therefor loosing track of the previously allocated
> rsa_candidate list.

Oh. That's lame. Sounds like rsa_candidates is really used only as
"temporary variable" in the ph1 handler so the data can be passed
between functions. There's a lot of ph1 handler members like that. IMHO,
those should not be there. Each function should just handle those
internally. But that's probably not going to happen before 0.8 release.

> In case you agree with my initial solution, I attached a slight
> correction of my initial patch, resetting the pointer. Using this patch,
> the breakpoint mentioned before is not being hit.

Yes. Sounds like this is the easy and proper fix for this then. I'll
commit it shortly.

Thanks,
  Timo


------------------------------------------------------------------------------
The ultimate all-in-one performance toolkit: Intel(R) Parallel Studio XE:
Pinpoint memory and threading errors before they happen.
Find and fix more than 250 security defects in the development cycle.
Locate bottlenecks in serial and parallel code that limit performance.
http://p.sf.net/sfu/intel-dev2devfeb
_______________________________________________
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]