Bug 31 - Reuse of Transaction ID can lead to assert in resiprocate 1.7
Summary: Reuse of Transaction ID can lead to assert in resiprocate 1.7
Status: NEW
Alias: None
Product: resiprocate
Classification: Unclassified
Component: stack (libresip) (show other bugs)
Version: unspecified
Hardware: Intel Linux
: P1 major
Assignee: Owner of all unassigned bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-07-17 09:54 CDT by John Gregg
Modified: 2012-07-17 09:54 CDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description John Gregg 2012-07-17 09:54:51 CDT
TU sends REGISTER to stack, stack creates TransactionState and sends REGISTER out on the wire. The other end never replies, leading to multiple retransmissions (Timer E1) followed by an eventual Timer F firing, which results in a 408 sent back up to the TU, and the immediate destruction of
the TransactionState. Note that Timer E1 is still running, and it is assumed that when it fires, it will try to look up the TransactionState by tid, find nothing, and expire harlessly with nothing to do. However, if at this point the TU (wrongly) sends an ACK with the same tid, the stack will create a whole new Stateless TransactionState. Now, when the Timer E1 fires, it will look up the
TransactionState based on tid, and instead of finding its old REGISTER TransactionState, it will find this new Stateless TransactionState, and we assert in TransactionState::processStateless(), isTimer(message), but timer->getType() != Timer::TimerStateless.

A simple fix for this particular situation is this: when Timer F fires, instead of immediately destroying the TransactionState, just put the TransactionState in Completed state and set Timer K, which will destroy the TransactionState five seconds from now. That way, when Timer E1 fires, it will find the old TransactionState. Likewise, the ACK will also be handled by the old TransactionState.

I think the larger problem, though, is zombie timers. When you destroy a TransactionState, there should be a way to cancel all outstanding timers associated with it. Since TransactionStates are hashed by tid, which is not under our control (it comes in the messages from elsewhere), we can't know that they won't be reused, and timers for old TransactionStates will end up finding totally unrelated new TransactionStates that happen to be using the same tid as the old ones.