Re: [reSIProcate] Dum segfaulting when dns is unavailable

[Scott Zuk <szuk@xxxxxxxxxxxxxxx>]

On January 13, 2005 4:30 pm, Scott Zuk wrote:
> Hi,
>
> I've found a problem with resiprocate that causes the stack to segfault
> when returning 503 errors caused by failed dns requests.  I've attached a
> short test program that demonstrates the failure.
...

Well, I don't know if anyone's looked at this but I've found the cause after 
some debugging.  Basically, the problem is caused by accessing memory in 
TransactionState::sendToWire() that was already deleted in 
TransactionState::processNoDnsResults().  

Here's a simple discription of the call flow when resolving addresses for 
outgoing client requests if a network connection is unavailable for some 
reason:

== sending out a new request that requires a dns lookup ==
TransactionController::process()
-> at line 89 TransactionState::process() is called
  TransactionState::process()
  -> at line 307 a new TransactionState object is created [1] 
  -> at line 309 TransactionState::processClientNonInvite() is called
    TransactionState::processClientNonInvite()
    -> at line 400 TransactionState::sendToWire() is called
      TransactionState::sendToWire()
      -> at line 1448 mController.mTransportSelector.dnsResolve() is called

At this point the call stack goes through DnsInterface, DnsResult, AresDns 
etc. to look up the required hostname through dns.  If there is no network 
available, all attempts will fail and DnsResult will call the 
TransactionState::handle() method of the same object created at [1] above and 
reply with a 503 message. The call stack then looks like the following:

== dns lookup returned no results ==
TransactionState::handle()
-> at line 1297 TransactionState::processNoDnsResults() is called
  TransactionState::processNoDnsResults()
  -> at line 1219 TransactionState::sendToTU() is called
  -> at line 1220 TransactionState::terminateClientTransaction() is called
  -> at line 1221 delete this is called. This is the same object that was 
created at [1] above.

Eventually the call stack works its way back to where dnsResolve was called in 
TransactionState::sendToWire().  But this puts us back into the 
TransactionState object we just deleted!  The following call to 
mDnsResult->available() refers to invalid memory and bad things happen.  
Eventually the stack crashes.

I've made the following small changes to TransactionState.cxx that I think 
fixes the problem without breaking things.  Can someone more familiar with 
the transaction state code take a look?

------------------------
In TransactionState::~TransactionState
if (mDnsResult)
{
   mDnsResult->destroy();
   mDnsResult = 0;
   ^^^^^^^^^^^^^^^ Set pointer to null at destruction
}

In TransactionState::processNoDnsResults
sendToTU(response); // !jf! should be 480?
terminateClientTransaction(mId);
//delete this;
^^^^^^^^^^^^^^ don't delete here anymore

In TransactionState::sendToWire
mController.mTransportSelector.dnsResolve(mDnsResult, sip);
assert(mDnsResult); // !ah! is this really an assertion or an error?

// do it now, if there is an immediate result
if (mDnsResult->available() == DnsResult::Available)
{
  handle(mDnsResult);
}
else if (mDnsResult->available() == DnsResult::Finished)
{
  delete this;
  return;
}    
^^^^^^^^^^^^^^^ add another case for Finished and delete here instead.
------------------------


Thanks,
~Scott