[h323plus] Double-delete Capability bug

Simon Horne s.horne at packetizer.com
Sat Jan 23 11:22:31 EST 2016


Jan

As I suspected, I just check with VLD on windows and the change causes a
9kB/call memory leak (as the capability gets cloned 3 times per call). Needs
to be reverted out.

Simon

-----Original Message-----
From: h323plus [mailto:h323plus-bounces at lists.packetizer.com] On Behalf Of
Jan Willamowius
Sent: Saturday, January 23, 2016 4:49 AM
To: h323plus at lists.packetizer.com
Subject: Re: [h323plus] Double-delete Capability bug

Hi Iurii,

I have added the Clone() in the CVS, but could you please check (wg.
with Valgrind) if we now have a memory leak in some cases instead of a crash
?

Regards,
Jan

--
Jan Willamowius, Founder of the GNU Gatekeeper Project EMail  :
jan at willamowius.de
Website: http://www.gnugk.org
Support: http://www.willamowius.com/gnugk-support.html

Relaxed Communications GmbH
Frahmredder 91
22393 Hamburg
Geschäftsführer: Jan Willamowius
HRB 125261 (Amtsgericht Hamburg)
USt-IdNr: DE286003584

Iurii Gordiienko wrote:
> Hi,
> I have found something... For
H323ExtendedVideoCapability::AddAllCapabilities function we are using
H323CodecExtendedVideoCapability::AddCapability function for
extCapability->AddCapability(*r).
H323CodecExtendedVideoCapability::AddCapability function uses
extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)) function.
The function H323ExtendedVideoFactory::CreateInstance(cap) will call "new"
if any same "cap" does not is exists for now< for other case it will return
the pointer to current actual Capability. But
H323CodecExtendedVideoCapability::extCapabilities will delete all items in
time of execution  extCapabilities.RemoveAll() function
from H323CodecExtendedVideoCapability::~H323CodecExtendedVideoCapability().
In result we have "double delete" and undefined behavior (crash for worst
case). 
> 
> --------------------------------
> void H323ExtendedVideoCapability::AddAllCapabilities(      
H323Capabilities & basecapabilities, PINDEX descriptorNum,PINDEX
simultaneous) {   H323ExtendedVideoFactory::KeyList_T extCaps =
H323ExtendedVideoFactory::GetKeyList();   if (extCaps.size() > 0) {    
H323CodecExtendedVideoCapability * capability = new
H323CodecExtendedVideoCapability();    
H323ExtendedVideoFactory::KeyList_T::const_iterator r;         PINDEX num =
P_MAX_INDEX;         for (r = extCaps.begin(); r != extCaps.end(); ++r) {  
         H323CodecExtendedVideoCapability * extCapability =
(H323CodecExtendedVideoCapability *)capability->Clone();          
 extCapability->AddCapability(*r);            num =
basecapabilities.SetCapability(descriptorNum, simultaneous,extCapability);  
         simultaneous = num;         }     simultaneous = P_MAX_INDEX;    
basecapabilities.SetCapability(descriptorNum, simultaneous,new
H323ControlExtendedVideoCapability());  !
>     delete capability;   }  }
> void H323CodecExtendedVideoCapability::AddCapability(const PString & 
> cap) {     
> extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)); }
> 
> -------------------------------------------------
> I propose to replace the "void
H323CodecExtendedVideoCapability::AddCapability(const PString & cap)" to
this one: 
> void H323CodecExtendedVideoCapability::AddCapability(const PString & 
> cap) {     
> extCapabilities.Add(H323ExtendedVideoFactory::CreateInstance(cap)->clo
> ne()); }
> 
> 
> 
> Thanks -- Iurii Gordiienko






More information about the h323plus mailing list