[asterisk-commits] russell: branch russell/iax2-frame-race-1.2 r118749 - /team/russell/iax2-fram...

[SVN commits to the Asterisk project]

Author: russell
Date: Wed May 28 11:31:29 2008
New Revision: 118749

URL: http://svn.digium.com/view/asterisk?view=rev&rev=118749
Log:
Fix a race condition that can cause an IAX2 call to end up in a storm of INVAL
and VNAK frames being sent.

The recent security fix was to make chan_iax2 do more strict checking
of incoming full frames.  Part of this fix was to ensure that all full frames
that were supposed to include the destination call number, did include it.
So, the 2nd frame received for a call is expected to include the destination
call number.  However, this is where a problem can occur.

The IAX2 channel driver schedules a PING and LAGRQ to be sent right after an
IAX2 exchange begins.  One of these frames (usually the LAGRQ) may be sent as
the second frame before the first frame is received from the other end.  If that
happens, then the frame will be rejected as invalid now, and for every time in
the future as it gets retransmitted.

I have made a minor change to the callbacks that send LAGRQ and PING to ensure
that they do not get sent unless the peer's call number is already known.

Modified:
    team/russell/iax2-frame-race-1.2/channels/chan_iax2.c

Modified: team/russell/iax2-frame-race-1.2/channels/chan_iax2.c
URL: http://svn.digium.com/view/asterisk/team/russell/iax2-frame-race-1.2/channels/chan_iax2.c?view=diff&rev=118749&r1=118748&r2=118749
==============================================================================
--- team/russell/iax2-frame-race-1.2/channels/chan_iax2.c (original)
+++ team/russell/iax2-frame-race-1.2/channels/chan_iax2.c Wed May 28 11:31:29 2008
@@ -806,15 +806,27 @@
 static int send_ping(void *data)
 {
 	int callno = (long)data;
+	int res = 0;
+
 	/* Ping only if it's real, not if it's bridged */
-	if (iaxs[callno]) {
+
+	ast_mutex_lock(&iaxsl[callno]);
+
+	while (iaxs[callno]) {
+		res = 1;
+		if (!iaxs[callno]->peercallno) {
+			break;
+		}
 #ifdef BRIDGE_OPTIMIZATION
 		if (!iaxs[callno]->bridgecallno)
 #endif
 			send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_PING, 0, NULL, 0, -1);
-		return 1;
-	} else
-		return 0;
+		break;
+	}
+
+	ast_mutex_unlock(&iaxsl[callno]);
+
+	return res;
 }
 
 static int get_encrypt_methods(const char *s)
@@ -832,15 +844,27 @@
 static int send_lagrq(void *data)
 {
 	int callno = (long)data;
+	int res = 0;
+
 	/* Ping only if it's real not if it's bridged */
-	if (iaxs[callno]) {
+
+	ast_mutex_lock(&iaxsl[callno]);
+
+	while (iaxs[callno]) {
+		res = 1;
+		if (!iaxs[callno]->peercallno) {
+			break;
+		}
 #ifdef BRIDGE_OPTIMIZATION
 		if (!iaxs[callno]->bridgecallno)
 #endif		
 			send_command(iaxs[callno], AST_FRAME_IAX, IAX_COMMAND_LAGRQ, 0, NULL, 0, -1);
-		return 1;
-	} else
-		return 0;
+		break;
+	}
+
+	ast_mutex_unlock(&iaxsl[callno]);
+
+	return res;
 }
 
 static unsigned char compress_subclass(int subclass)