[asterisk-commits] twilson: branch group/srtp_reboot r244449 - in /team/group/srtp_reboot: ./ ch...

SVN commits to the Asterisk project asterisk-commits at lists.digium.com
Tue Feb 2 16:47:12 CST 2010


Author: twilson
Date: Tue Feb  2 16:47:08 2010
New Revision: 244449

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=244449
Log:
Merged revisions 244443 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/trunk

........
  r244443 | dvossel | 2010-02-02 16:27:23 -0600 (Tue, 02 Feb 2010) | 18 lines
  
  fixes crash during T.38 negotiation caused by invalid or missing FaxMaxDatagram field
  
  AST-2010-001
  
  (closes issue #16634)
  Reported by: krn
  
  (closes issue #16724)
  Reported by: barthpbx
  
  (closes issue #16517)
  Reported by: bklang
  
  (closes issue #16485)
  Reported by: elsto
........

Modified:
    team/group/srtp_reboot/   (props changed)
    team/group/srtp_reboot/channels/chan_sip.c
    team/group/srtp_reboot/include/asterisk/udptl.h
    team/group/srtp_reboot/main/udptl.c

Propchange: team/group/srtp_reboot/
------------------------------------------------------------------------------
--- svnmerge-integrated (original)
+++ svnmerge-integrated Tue Feb  2 16:47:08 2010
@@ -1,1 +1,1 @@
-/trunk:1-244440
+/trunk:1-244448

Modified: team/group/srtp_reboot/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/team/group/srtp_reboot/channels/chan_sip.c?view=diff&rev=244449&r1=244448&r2=244449
==============================================================================
--- team/group/srtp_reboot/channels/chan_sip.c (original)
+++ team/group/srtp_reboot/channels/chan_sip.c Tue Feb  2 16:47:08 2010
@@ -4067,7 +4067,7 @@
 	p->packets = pkt;	/* Add it to the queue */
 	if (resp) {
 		/* Parse out the response code */
-		if (sscanf(ast_str_buffer(pkt->data), "SIP/2.0 %30d", &respid) == 1) {
+		if (sscanf(ast_str_buffer(pkt->data), "SIP/2.0 %30u", &respid) == 1) {
 			pkt->response_code = respid;
 		}
 	}
@@ -5301,20 +5301,30 @@
 		return;
 
 	/* Given the state requested and old state determine what control frame we want to queue up */
-	if (state == T38_PEER_REINVITE) {
+	switch (state) {
+	case T38_PEER_REINVITE:
 		parameters = p->t38.their_parms;
 		parameters.max_ifp = ast_udptl_get_far_max_ifp(p->udptl);
 		parameters.request_response = AST_T38_REQUEST_NEGOTIATE;
 		ast_udptl_set_tag(p->udptl, "SIP/%s", p->username);
-	} else if (state == T38_ENABLED) {
+		break;
+	case T38_ENABLED:
 		parameters = p->t38.their_parms;
 		parameters.max_ifp = ast_udptl_get_far_max_ifp(p->udptl);
 		parameters.request_response = AST_T38_NEGOTIATED;
 		ast_udptl_set_tag(p->udptl, "SIP/%s", p->username);
-	} else if (state == T38_DISABLED && old == T38_ENABLED)
-		parameters.request_response = AST_T38_TERMINATED;
-	else if (state == T38_DISABLED && old == T38_LOCAL_REINVITE)
-		parameters.request_response = AST_T38_REFUSED;
+		break;
+	case T38_DISABLED:
+		if (old == T38_ENABLED) {
+			parameters.request_response = AST_T38_TERMINATED;
+		} else if (old == T38_LOCAL_REINVITE) {
+			parameters.request_response = AST_T38_REFUSED;
+		}
+		break;
+	case T38_LOCAL_REINVITE:
+		/* wait until we get a peer response before responding to local reinvite */
+		break;
+	}
 
 	/* Woot we got a message, create a control frame and send it on! */
 	if (parameters.request_response)
@@ -6745,10 +6755,21 @@
 /*! \brief Helper function which updates T.38 capability information and triggers a reinvite */
 static void interpret_t38_parameters(struct sip_pvt *p, const struct ast_control_t38_parameters *parameters)
 {
+	if (!ast_test_flag(&p->flags[1], SIP_PAGE2_T38SUPPORT)) {
+		return;
+	}
 	switch (parameters->request_response) {
 	case AST_T38_NEGOTIATED:
 	case AST_T38_REQUEST_NEGOTIATE:         /* Request T38 */
-		if (p->t38.state == T38_PEER_REINVITE) {
+		/* Negotiation can not take place without a valid max_ifp value. */
+		if (!parameters->max_ifp) {
+				change_t38_state(p, T38_DISABLED);
+				if (p->t38.state == T38_PEER_REINVITE) {
+					AST_SCHED_DEL_UNREF(sched, p->t38id, dialog_unref(p, "when you delete the t38id sched, you should dec the refcount for the stored dialog ptr"));
+					transmit_response_reliable(p, "488 Not acceptable here", &p->initreq);
+				}
+				break;
+		} else if (p->t38.state == T38_PEER_REINVITE) {
 			AST_SCHED_DEL_UNREF(sched, p->t38id, dialog_unref(p, "when you delete the t38id sched, you should dec the refcount for the stored dialog ptr"));
 			p->t38.our_parms = *parameters;
 			/* modify our parameters to conform to the peer's parameters,
@@ -6768,7 +6789,7 @@
 			ast_udptl_set_local_max_ifp(p->udptl, p->t38.our_parms.max_ifp);
 			change_t38_state(p, T38_ENABLED);
 			transmit_response_with_t38_sdp(p, "200 OK", &p->initreq, XMIT_CRITICAL);
-		} else if (ast_test_flag(&p->flags[1], SIP_PAGE2_T38SUPPORT) && p->t38.state != T38_ENABLED) {
+		} else if (p->t38.state != T38_ENABLED) {
 			p->t38.our_parms = *parameters;
 			ast_udptl_set_local_max_ifp(p->udptl, p->t38.our_parms.max_ifp);
 			change_t38_state(p, T38_LOCAL_REINVITE);
@@ -8318,10 +8339,10 @@
 	}
 	/* We only want the m and c lines for audio */
 	for (m = get_sdp_iterate(&miterator, req, "m"); !ast_strlen_zero(m); m = get_sdp_iterate(&miterator, req, "m")) {
-		if ((media == SDP_AUDIO && ((sscanf(m, "audio %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-		    (sscanf(m, "audio %30d RTP/AVP %n", &x, &len) == 1 && len > 0))) ||
-			(media == SDP_VIDEO && ((sscanf(m, "video %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-		    (sscanf(m, "video %30d RTP/AVP %n", &x, &len) == 1 && len > 0)))) {
+		if ((media == SDP_AUDIO && ((sscanf(m, "audio %30u/%30u RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+		    (sscanf(m, "audio %30u RTP/AVP %n", &x, &len) == 1 && len > 0))) ||
+			(media == SDP_VIDEO && ((sscanf(m, "video %30u/%30u RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+		    (sscanf(m, "video %30u RTP/AVP %n", &x, &len) == 1 && len > 0)))) {
 			/* See if there's a c= line for this media stream.
 			 * XXX There is no guarantee that we'll be grabbing the c= line for this
 			 * particular media stream here. However, this is the same logic used in process_sdp.
@@ -8509,8 +8530,8 @@
 		nextm = get_sdp_iterate(&next, req, "m");
 
 		/* Search for audio media definition */
-		if ((sscanf(m, "audio %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-		    (sscanf(m, "audio %30d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
+		if ((sscanf(m, "audio %30u/%30u RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+		    (sscanf(m, "audio %30u RTP/AVP %n", &x, &len) == 1 && len > 0)) {
 			audio = TRUE;
 			p->offered_media[SDP_AUDIO].offered = TRUE;
 			numberofmediastreams++;
@@ -8520,7 +8541,7 @@
 			codecs = m + len;
 			ast_copy_string(p->offered_media[SDP_AUDIO].codecs, codecs, sizeof(p->offered_media[SDP_AUDIO].codecs));
 			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
-				if (sscanf(codecs, "%30d%n", &codec, &len) != 1) {
+				if (sscanf(codecs, "%30u%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
 				}
@@ -8530,8 +8551,8 @@
 				ast_rtp_codecs_payloads_set_m_type(&newaudiortp, NULL, codec);
 			}
 		/* Search for video media definition */
-		} else if ((sscanf(m, "video %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-			   (sscanf(m, "video %30d RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
+		} else if ((sscanf(m, "video %30u/%30u RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+			   (sscanf(m, "video %30u RTP/AVP %n", &x, &len) == 1 && len >= 0)) {
 			video = TRUE;
 			p->novideo = FALSE;
 			p->offered_media[SDP_VIDEO].offered = TRUE;
@@ -8542,7 +8563,7 @@
 			codecs = m + len;
 			ast_copy_string(p->offered_media[SDP_VIDEO].codecs, codecs, sizeof(p->offered_media[SDP_VIDEO].codecs));
 			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
-				if (sscanf(codecs, "%30d%n", &codec, &len) != 1) {
+				if (sscanf(codecs, "%30u%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
 				}
@@ -8551,8 +8572,8 @@
 				ast_rtp_codecs_payloads_set_m_type(&newvideortp, NULL, codec);
 			}
 		/* Search for text media definition */
-		} else if ((sscanf(m, "text %30d/%30d RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
-			   (sscanf(m, "text %30d RTP/AVP %n", &x, &len) == 1 && len > 0)) {
+		} else if ((sscanf(m, "text %30u/%30u RTP/AVP %n", &x, &numberofports, &len) == 2 && len > 0) ||
+			   (sscanf(m, "text %30u RTP/AVP %n", &x, &len) == 1 && len > 0)) {
 			text = TRUE;
 			p->notext = FALSE;
 			p->offered_media[SDP_TEXT].offered = TRUE;
@@ -8563,7 +8584,7 @@
 			codecs = m + len;
 			ast_copy_string(p->offered_media[SDP_TEXT].codecs, codecs, sizeof(p->offered_media[SDP_TEXT].codecs));
 			for (; !ast_strlen_zero(codecs); codecs = ast_skip_blanks(codecs + len)) {
-				if (sscanf(codecs, "%30d%n", &codec, &len) != 1) {
+				if (sscanf(codecs, "%30u%n", &codec, &len) != 1) {
 					ast_log(LOG_WARNING, "Error in codec string '%s'\n", codecs);
 					return -1;
 				}
@@ -8572,8 +8593,8 @@
 				ast_rtp_codecs_payloads_set_m_type(&newtextrtp, NULL, codec);
 			}
 		/* Search for image media definition */
-		} else if (p->udptl && ((sscanf(m, "image %30d udptl t38%n", &x, &len) == 1 && len > 0) ||
-					(sscanf(m, "image %30d UDPTL t38%n", &x, &len) == 1 && len > 0) )) {
+		} else if (p->udptl && ((sscanf(m, "image %30u udptl t38%n", &x, &len) == 1 && len > 0) ||
+					(sscanf(m, "image %30u UDPTL t38%n", &x, &len) == 1 && len > 0) )) {
 			image = TRUE;
 			if (debug)
 				ast_verbose("Got T.38 offer in SDP in dialog %s\n", p->callid);
@@ -8819,6 +8840,12 @@
 			if (debug)
 				ast_debug(1,"Peer T.38 UDPTL is at port %s:%d\n", ast_inet_ntoa(isin.sin_addr), ntohs(isin.sin_port));
 
+			/* verify the far max ifp can be calculated. this requires far max datagram to be set. */
+			if (!ast_udptl_get_far_max_datagram(p->udptl)) {
+				/* setting to zero will force a default if none was provided by the SDP */
+				ast_udptl_set_far_max_datagram(p->udptl, 0);
+			}
+
 			/* Remote party offers T38, we need to update state */
 			if ((t38action == SDP_T38_ACCEPT) &&
 			    (p->t38.state == T38_LOCAL_REINVITE)) {
@@ -9201,12 +9228,12 @@
 {
 	int found = FALSE;
 	char s[256];
-	int x;
-
-	if ((sscanf(a, "T38FaxMaxBuffer:%30d", &x) == 1)) {
+	unsigned int x;
+
+	if ((sscanf(a, "T38FaxMaxBuffer:%30u", &x) == 1)) {
 		ast_debug(3, "MaxBufferSize:%d\n", x);
 		found = TRUE;
-	} else if ((sscanf(a, "T38MaxBitRate:%30d", &x) == 1) || (sscanf(a, "T38FaxMaxRate:%30d", &x) == 1)) {
+	} else if ((sscanf(a, "T38MaxBitRate:%30u", &x) == 1) || (sscanf(a, "T38FaxMaxRate:%30u", &x) == 1)) {
 		ast_debug(3, "T38MaxBitRate: %d\n", x);
 		switch (x) {
 		case 14400:
@@ -9229,21 +9256,21 @@
 			break;
 		}
 		found = TRUE;
-	} else if ((sscanf(a, "T38FaxVersion:%30d", &x) == 1)) {
-		ast_debug(3, "FaxVersion: %d\n", x);
+	} else if ((sscanf(a, "T38FaxVersion:%30u", &x) == 1)) {
+		ast_debug(3, "FaxVersion: %u\n", x);
 		p->t38.their_parms.version = x;
 		found = TRUE;
-	} else if ((sscanf(a, "T38FaxMaxDatagram:%30d", &x) == 1) || (sscanf(a, "T38MaxDatagram:%30d", &x) == 1)) {
+	} else if ((sscanf(a, "T38FaxMaxDatagram:%30u", &x) == 1) || (sscanf(a, "T38MaxDatagram:%30u", &x) == 1)) {
 		/* override the supplied value if the configuration requests it */
 		if (p->t38_maxdatagram > x) {
 			ast_debug(1, "Overriding T38FaxMaxDatagram '%d' with '%d'\n", x, p->t38_maxdatagram);
 			x = p->t38_maxdatagram;
 		}
-		ast_debug(3, "FaxMaxDatagram: %d\n", x);
+		ast_debug(3, "FaxMaxDatagram: %u\n", x);
 		ast_udptl_set_far_max_datagram(p->udptl, x);
 		found = TRUE;
 	} else if ((strncmp(a, "T38FaxFillBitRemoval", 20) == 0)) {
-		if (sscanf(a, "T38FaxFillBitRemoval:%30d", &x) == 1) {
+		if (sscanf(a, "T38FaxFillBitRemoval:%30u", &x) == 1) {
 			ast_debug(3, "FillBitRemoval: %d\n", x);
 			if (x == 1) {
 				p->t38.their_parms.fill_bit_removal = TRUE;
@@ -9254,7 +9281,7 @@
 		}
 		found = TRUE;
 	} else if ((strncmp(a, "T38FaxTranscodingMMR", 20) == 0)) {
-		if (sscanf(a, "T38FaxTranscodingMMR:%30d", &x) == 1) {
+		if (sscanf(a, "T38FaxTranscodingMMR:%30u", &x) == 1) {
 			ast_debug(3, "Transcoding MMR: %d\n", x);
 			if (x == 1) {
 				p->t38.their_parms.transcoding_mmr = TRUE;
@@ -9265,7 +9292,7 @@
 		}
 		found = TRUE;
 	} else if ((strncmp(a, "T38FaxTranscodingJBIG", 21) == 0)) {
-		if (sscanf(a, "T38FaxTranscodingJBIG:%30d", &x) == 1) {
+		if (sscanf(a, "T38FaxTranscodingJBIG:%30u", &x) == 1) {
 			ast_debug(3, "Transcoding JBIG: %d\n", x);
 			if (x == 1) {
 				p->t38.their_parms.transcoding_jbig = TRUE;
@@ -10770,7 +10797,7 @@
 			ast_str_append(&a_modem, 0, "a=T38FaxRateManagement:localTCF\r\n");
 			break;
 		}
-		ast_str_append(&a_modem, 0, "a=T38FaxMaxDatagram:%d\r\n", ast_udptl_get_local_max_datagram(p->udptl));
+		ast_str_append(&a_modem, 0, "a=T38FaxMaxDatagram:%u\r\n", ast_udptl_get_local_max_datagram(p->udptl));
 		switch (ast_udptl_get_error_correction_scheme(p->udptl)) {
 		case UDPTL_ERROR_CORRECTION_NONE:
 			break;
@@ -24447,7 +24474,7 @@
 				ast_clear_flag(&flags[1], SIP_PAGE2_T38SUPPORT);
 				ast_set_flag(&flags[1], SIP_PAGE2_T38SUPPORT_UDPTL);
 			} else if (!strncasecmp(word, "maxdatagram=", 12)) {
-				if (sscanf(&word[12], "%30d", maxdatagram) != 1) {
+				if (sscanf(&word[12], "%30u", maxdatagram) != 1) {
 					ast_log(LOG_WARNING, "Invalid maxdatagram '%s' at line %d of %s\n", v->value, v->lineno, config);
 					*maxdatagram = global_t38_maxdatagram;
 				}

Modified: team/group/srtp_reboot/include/asterisk/udptl.h
URL: http://svnview.digium.com/svn/asterisk/team/group/srtp_reboot/include/asterisk/udptl.h?view=diff&rev=244449&r1=244448&r2=244449
==============================================================================
--- team/group/srtp_reboot/include/asterisk/udptl.h (original)
+++ team/group/srtp_reboot/include/asterisk/udptl.h Tue Feb  2 16:47:08 2010
@@ -108,12 +108,28 @@
 
 void ast_udptl_set_local_max_ifp(struct ast_udptl *udptl, unsigned int max_ifp);
 
+/*! 
+ * \brief retrieves local_max_datagram.
+ * 
+ * \retval positive value representing max datagram size.
+ * \retval 0 if no value is present
+ */
 unsigned int ast_udptl_get_local_max_datagram(struct ast_udptl *udptl);
 
+/*! 
+ * \brief sets far max datagram size.  If max_datagram is = 0, the far max datagram
+ *  size is set to a default value.
+ */
 void ast_udptl_set_far_max_datagram(struct ast_udptl *udptl, unsigned int max_datagram);
 
 unsigned int ast_udptl_get_far_max_datagram(const struct ast_udptl *udptl);
 
+/*! 
+ * \brief retrieves far max ifp
+ * 
+ * \retval positive value representing max ifp size
+ * \retval 0 if no value is present
+ */
 unsigned int ast_udptl_get_far_max_ifp(struct ast_udptl *udptl);
 
 void ast_udptl_setnat(struct ast_udptl *udptl, int nat);

Modified: team/group/srtp_reboot/main/udptl.c
URL: http://svnview.digium.com/svn/asterisk/team/group/srtp_reboot/main/udptl.c?view=diff&rev=244449&r1=244448&r2=244449
==============================================================================
--- team/group/srtp_reboot/main/udptl.c (original)
+++ team/group/srtp_reboot/main/udptl.c Tue Feb  2 16:47:08 2010
@@ -91,6 +91,8 @@
 static int use_even_ports;
 
 #define LOCAL_FAX_MAX_DATAGRAM      1400
+#define DEFAULT_FAX_MAX_DATAGRAM    400
+#define FAX_MAX_DATAGRAM_LIMIT      1400
 #define MAX_FEC_ENTRIES             5
 #define MAX_FEC_SPAN                5
 
@@ -861,9 +863,13 @@
 
 void ast_udptl_set_local_max_ifp(struct ast_udptl *udptl, unsigned int max_ifp)
 {
-	udptl->local_max_ifp = max_ifp;
-	/* reset calculated values so they'll be computed again */
-	udptl->local_max_datagram = -1;
+	/* make sure max_ifp is a positive value since a cast will take place when
+	 * when setting local_max_ifp */
+	if ((signed int) max_ifp > 0) {
+		udptl->local_max_ifp = max_ifp;
+		/* reset calculated values so they'll be computed again */
+		udptl->local_max_datagram = -1;
+	}
 }
 
 unsigned int ast_udptl_get_local_max_datagram(struct ast_udptl *udptl)
@@ -871,18 +877,30 @@
 	if (udptl->local_max_datagram == -1) {
 		calculate_local_max_datagram(udptl);
 	}
+
+	/* this function expects a unsigned value in return. */
+	if (udptl->local_max_datagram < 0) {
+		return 0;
+	}
 	return udptl->local_max_datagram;
 }
 
 void ast_udptl_set_far_max_datagram(struct ast_udptl *udptl, unsigned int max_datagram)
 {
-	udptl->far_max_datagram = max_datagram;
+	if (!max_datagram || (max_datagram > FAX_MAX_DATAGRAM_LIMIT)) {
+		udptl->far_max_datagram = DEFAULT_FAX_MAX_DATAGRAM;
+	} else {
+		udptl->far_max_datagram = max_datagram;
+	}
 	/* reset calculated values so they'll be computed again */
 	udptl->far_max_ifp = -1;
 }
 
 unsigned int ast_udptl_get_far_max_datagram(const struct ast_udptl *udptl)
 {
+	if (udptl->far_max_datagram < 0) {
+		return 0;
+	}
 	return udptl->far_max_datagram;
 }
 
@@ -890,6 +908,10 @@
 {
 	if (udptl->far_max_ifp == -1) {
 		calculate_far_max_ifp(udptl);
+	}
+
+	if (udptl->far_max_ifp < 0) {
+		return 0;
 	}
 	return udptl->far_max_ifp;
 }
@@ -1040,7 +1062,11 @@
 	unsigned int seq;
 	unsigned int len = f->datalen;
 	int res;
-	uint8_t buf[s->far_max_datagram];
+	/* if no max datagram size is provided, use default value */
+	const int bufsize = (s->far_max_datagram > 0) ? s->far_max_datagram : DEFAULT_FAX_MAX_DATAGRAM;
+	uint8_t buf[bufsize];
+
+	memset(buf, 0, sizeof(buf));
 
 	/* If we have no peer, return immediately */	
 	if (s->them.sin_addr.s_addr == INADDR_ANY)




More information about the asterisk-commits mailing list