[svn-commits] mjordan: trunk r356216 - in /trunk: ./ channels/chan_sip.c

[SVN commits to the Digium repositories]

Author: mjordan
Date: Wed Feb 22 08:54:42 2012
New Revision: 356216

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=356216
Log:
Merged revisions 356215 via svnmerge from 
https://origsvn.digium.com/svn/asterisk/branches/10

................
  r356215 | mjordan | 2012-02-22 08:53:53 -0600 (Wed, 22 Feb 2012) | 32 lines
  
  Merged revisions 356214 via svnmerge from 
  https://origsvn.digium.com/svn/asterisk/branches/1.8
  
  ........
    r356214 | mjordan | 2012-02-22 08:50:20 -0600 (Wed, 22 Feb 2012) | 27 lines
    
    Fix potential buffer overrun and memory leak when executing "sip show peers"
    
    The "sip show peers" command uses a fix sized array to sort the current peers
    in the peers ao2_container.  The size of the array is based on the current
    number of peers in the container.  However, once the size of the array is
    determined, the number of peers in the container can change, as the peers
    container is not locked.  This could cause a buffer overrun when populating
    the array, if peers were added to the container after the array was created.
    Additionally, a memory leak of the allocated array would occur if a user
    caused the _show_peers method to return CLI_SHOWUSAGE.
    
    We now create a snapshot of the current peers using an ao2_callback with the
    OBJ_MULTIPLE flag.  This size of the array is set to the number of peers
    that the iterator will iterate over; hence, if peers are added or removed
    from the peers container it will not affect the execution of the "sip show
    peers" command.
    
    Review: https://reviewboard.asterisk.org/r/1738/
    
    (closes issue ASTERISK-19231)
    (closes issue ASTERISK-19361)
    Reported by: Thomas Arimont, Jamuel Starkey
    Tested by: Thomas Arimont, Jamuel Starkey
    Patches: sip_show_peers_2012_02_16.diff uploaded by mjordan (license 6283)
  ........
................

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

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

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=356216&r1=356215&r2=356216
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Wed Feb 22 08:54:42 2012
@@ -17367,7 +17367,7 @@
 	regex_t regexbuf;
 	int havepattern = FALSE;
 	struct sip_peer *peer;
-	struct ao2_iterator i;
+	struct ao2_iterator* it_peers;
 
 /* the last argument is left-aligned, so we don't need a size anyways */
 #define FORMAT2 "%-25.25s %-39.39s %-3.3s %-10.10s %-3.3s %-8s %-11s %-32.32s %s\n"
@@ -17381,12 +17381,10 @@
 	const char *id;
 	char idtext[256] = "";
 	int realtimepeers;
-	int objcount = ao2_container_count(peers);
 	struct sip_peer **peerarray;
 	int k;
 
 	realtimepeers = ast_check_realtime("sippeers");
-	peerarray = ast_calloc(sizeof(struct sip_peer *), objcount);
 
 	if (s) {	/* Manager - get ActionID */
 		id = astman_get_header(m, "ActionID");
@@ -17408,11 +17406,26 @@
 		return CLI_SHOWUSAGE;
 	}
 
-	if (!s) /* Normal list */
+	if (!s) {
+		/* Normal list */
 		ast_cli(fd, FORMAT2, "Name/username", "Host", "Dyn", "Forcerport", "ACL", "Port", "Status", "Description", (realtimepeers ? "Realtime" : ""));
-
-	i = ao2_iterator_init(peers, 0);
-	while ((peer = ao2_t_iterator_next(&i, "iterate thru peers table"))) {
+	}
+
+	ao2_lock(peers);
+	if (!(it_peers = ao2_callback(peers, OBJ_MULTIPLE, NULL, NULL))) {
+		ast_log(AST_LOG_ERROR, "Unable to create iterator for peers container for sip show peers\n");
+		ao2_unlock(peers);
+		return CLI_FAILURE;
+	}
+	if (!(peerarray = ast_calloc(sizeof(struct sip_peer *), ao2_container_count(peers)))) {
+		ast_log(AST_LOG_ERROR, "Unable to allocate peer array for sip show peers\n");
+		ao2_iterator_destroy(it_peers);
+		ao2_unlock(peers);
+		return CLI_FAILURE;
+	}
+	ao2_unlock(peers);
+
+	while ((peer = ao2_t_iterator_next(it_peers, "iterate thru peers table"))) {
 		ao2_lock(peer);
 
 		if (!(peer->type & SIP_TYPE_PEER)) {
@@ -17422,7 +17435,6 @@
 		}
 
 		if (havepattern && regexec(&regexbuf, peer->name, 0, NULL, 0)) {
-			objcount--;
 			ao2_unlock(peer);
 			sip_unref_peer(peer, "toss iterator peer ptr before continue");
 			continue;
@@ -17431,7 +17443,7 @@
 		peerarray[total_peers++] = peer;
 		ao2_unlock(peer);
 	}
-	ao2_iterator_destroy(&i);
+	ao2_iterator_destroy(it_peers);
 
 	qsort(peerarray, total_peers, sizeof(struct sip_peer *), peercomparefunc);