[asterisk-commits] kmoore: branch 10 r371545 - in /branches/10: ./ main/udptl.c

[SVN commits to the Asterisk project]

Author: kmoore
Date: Mon Aug 20 10:27:15 2012
New Revision: 371545

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=371545
Log:
Ignore recovered zero-length secondary UDPTL packets

In some cases, recovering lost packets using the secondary packet
recovery mechanism with UDPTL/T.38 can result in the recovery of
zero-length packets. These must be ignored or the frame generated from
them can cause segfaults and allocation failures.

(closes issue ASTERISK-19762)
(closes issue ASTERISK-19373)
Reported-by: Benjamin (bulkorok)
Reported-by: Rob Gagnon (rgagnon)
........

Merged revisions 371544 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/10/   (props changed)
    branches/10/main/udptl.c

Propchange: branches/10/
------------------------------------------------------------------------------
Binary property 'branch-1.8-merged' - no diff available.

Modified: branches/10/main/udptl.c
URL: http://svnview.digium.com/svn/asterisk/branches/10/main/udptl.c?view=diff&rev=371545&r1=371544&r2=371545
==============================================================================
--- branches/10/main/udptl.c (original)
+++ branches/10/main/udptl.c Mon Aug 20 10:27:15 2012
@@ -315,16 +315,7 @@
 	int stat1;
 	int stat2;
 	int i;
-	int j;
-	int k;
-	int l;
-	int m;
-	int x;
-	int limit;
-	int which;
-	unsigned int ptr;
-	unsigned int count;
-	int total_count;
+	unsigned int ptr;	/* an index that keeps track of how much of the UDPTL packet has been processed */
 	int seq_no;
 	const uint8_t *ifp = NULL;
 	const uint8_t *data = NULL;
@@ -357,13 +348,21 @@
 		if (seq_no > s->rx_seq_no) {
 			/* We received a later packet than we expected, so we need to check if we can fill in the gap from the
 			   secondary packets. */
-			total_count = 0;
+			int total_count = 0;
 			do {
+				unsigned int count;
 				if ((stat2 = decode_length(buf, len, &ptr, &count)) < 0)
 					return -1;
 				for (i = 0; i < count && total_count + i < ARRAY_LEN(bufs); i++) {
-					if ((stat1 = decode_open_type(buf, len, &ptr, &bufs[total_count + i], &lengths[total_count + i])) != 0)
+					if ((stat1 = decode_open_type(buf, len, &ptr, &bufs[total_count + i], &lengths[total_count + i])) != 0) {
 						return -1;
+					}
+					/* valid secondaries can contain zero-length packets that should be ignored */
+					if (!bufs[total_count + i] || !lengths[total_count + i]) {
+						/* drop the count of items to process and reuse the buffers that were just set */
+						i--;
+						count--;
+					}
 				}
 				total_count += i;
 			}
@@ -373,7 +372,7 @@
 				if (seq_no - i >= s->rx_seq_no) {
 					/* This one wasn't seen before */
 					/* Decode the secondary IFP packet */
-					//fprintf(stderr, "Secondary %d, len %d\n", seq_no - i, lengths[i - 1]);
+					ast_debug(3, "Recovering lost packet via secondary %d, len %d\n", seq_no - i, lengths[i - 1]);
 					s->f[ifp_no].frametype = AST_FRAME_MODEM;
 					s->f[ifp_no].subclass.integer = AST_MODEM_T38;
 
@@ -393,6 +392,9 @@
 	}
 	else
 	{
+		int j;
+		int l;
+		int x;
 		/* FEC mode for error recovery */
 		/* Our buffers cannot tolerate overlength IFP packets in FEC mode */
 		if (ifp_len > LOCAL_FAX_MAX_DATAGRAM)
@@ -455,10 +457,13 @@
 		/* See if we can reconstruct anything which is missing */
 		/* TODO: this does not comprehensively hunt back and repair everything that is possible */
 		for (l = x; l != ((x - (16 - span*entries)) & UDPTL_BUF_MASK); l = (l - 1) & UDPTL_BUF_MASK) {
+			int m;
 			if (s->rx[l].fec_len[0] <= 0)
 				continue;
 			for (m = 0; m < s->rx[l].fec_entries; m++) {
-				limit = (l + m) & UDPTL_BUF_MASK;
+				int k;
+				int which;
+				int limit = (l + m) & UDPTL_BUF_MASK;
 				for (which = -1, k = (limit - s->rx[l].fec_span * s->rx[l].fec_entries) & UDPTL_BUF_MASK; k != limit; k = (k + s->rx[l].fec_entries) & UDPTL_BUF_MASK) {
 					if (s->rx[k].buf_len <= 0)
 						which = (which == -1) ? k : -2;