aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorrussell <russell@f38db490-d61c-443f-a65b-d21fe96a405b>2008-05-08 19:14:04 +0000
committerrussell <russell@f38db490-d61c-443f-a65b-d21fe96a405b>2008-05-08 19:14:04 +0000
commit332b5a375954e389ab3bf75484a50e6a3f3288d4 (patch)
treee43b1d9570277c1e2e9c8fd0ba719184e948500b
parentc7cadad16d1877a54acded5784b9132431efd348 (diff)
Fix a race condition that bbryant just found while doing some IAX2 testing.
He was running Asterisk trunk running IAX2 calls through a few Asterisk boxes, however, the audio was extremely choppy. We looked at a packet trace and saw a storm of INVAL and VNAK frames being sent from one box to another. It turned out that what had happened was that one box tried to send a CONTROL frame before the 3 way handshake had completed. So, that frame did not include the destination call number, because it didn't have it yet. Part of our recent work for security issues included an additional check to ensure that frames that are supposed to include the destination call number have the correct one. This caused the frame to be rejected with an INVAL. The frame would get retransmitted for forever, rejected every time ... This race condition exists in all versions that got the security changes, in theory. However, it is really only likely that this would cause a problem in Asterisk trunk. There was a control frame being sent (SRCUPDATE) at the _very_ beginning of the call, which does not exist in 1.2 or 1.4. However, I am fixing all versions that could potentially be affected by the introduced race condition. These changes are what bbryant and I came up with to fix the issue. Instead of simply dropping control frames that get sent before the handshake is complete, the code attempts to wait a little while, since in most cases, the handshake will complete very quickly. If it doesn't complete after yielding for a little while, then the frame gets dropped. git-svn-id: http://svn.digium.com/svn/asterisk/branches/1.2@115564 f38db490-d61c-443f-a65b-d21fe96a405b
-rw-r--r--channels/chan_iax2.c34
1 files changed, 33 insertions, 1 deletions
diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index 5d924c3d9..8e80a0f9c 100644
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -3553,9 +3553,41 @@ static int iax2_answer(struct ast_channel *c)
static int iax2_indicate(struct ast_channel *c, int condition)
{
unsigned short callno = PTR_TO_CALLNO(c->tech_pvt);
+ struct chan_iax2_pvt *pvt;
+ int res;
+
if (option_debug && iaxdebug)
ast_log(LOG_DEBUG, "Indicating condition %d\n", condition);
- return send_command_locked(callno, AST_FRAME_CONTROL, condition, 0, NULL, 0, -1);
+
+ ast_mutex_lock(&iaxsl[callno]);
+
+ pvt = iaxs[callno];
+
+ if (!pvt->peercallno) {
+ /* We don't know the remote side's call number, yet. :( */
+ int count = 10;
+ while (count-- && pvt && !pvt->peercallno) {
+ ast_mutex_unlock(&iaxsl[callno]);
+ usleep(1);
+ ast_mutex_lock(&iaxsl[callno]);
+ pvt = iaxs[callno];
+ }
+
+ if (!pvt->peercallno) {
+ /* Even after waiting, we still haven't completed a handshake with
+ * our peer, so we can't send this frame. Bail out. */
+ res = -1;
+ goto done;
+ }
+ }
+
+
+ res = send_command(iaxs[callno], AST_FRAME_CONTROL, condition, 0, NULL, 0, -1);
+
+done:
+ ast_mutex_unlock(&iaxsl[callno]);
+
+ return res;
}
static int iax2_transfer(struct ast_channel *c, const char *dest)