Review Board 1.7.16


chan_iax2 - unprotected access of iaxs[peer->callno] potentially results in segfault

Review Request #4599 - Created April 7, 2015 and submitted

Jaco Kroon
trunk
ASTERISK-21211
Reviewers
asterisk-dev
Asterisk
chan_iax2.c, specifically in function iax2_poke_peer, a completely unprotected access to iaxs[peer->callno] is made. Specifically I had a segfault trigger on line 12230, an access to iaxs[peer->callno] - the second in a sequence, so peer->callno can definitely change between the two

It is my understanding that:

1. peer->callno can change outside of the function , thus it's probably unsafe to use the raw value as per lines 12223, 12229 and 12230. I believe this should be callno, and not peer->callno. Please correct me if I'm wrong. This can either happen by us calling iax2_destroy, or simply another thread also scheduling a POKE on the same peer.

2. All reads and writes to iaxs[X] should be protected by a lock of iaxsl[X]. Lines 12229 and 12230 violates this currently.

I suspect my crash resulted from a sequence where a POKE was in process of being scheduled, another thread then called iax2_poke_peer for the same peer, called iax2_destroy on the iaxs[] busy being set up, and boom major catastrophe.
Been running since 11.2.1 with this patch, specifically 6/3/2013 (more than two years) in multiple production environments.  Many fewer segfaults.

Diff revision 1 (Latest)

  1. http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_iax2.c: Loading...
http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_iax2.c
Revision 434210 New Change
[20] 12322 lines
[+20] [+] static int iax2_poke_peer(struct iax2_peer *peer, int heldcall)
12323
	if (heldcall)
12323
	if (heldcall)
12324
		ast_mutex_unlock(&iaxsl[heldcall]);
12324
		ast_mutex_unlock(&iaxsl[heldcall]);
12325
	callno = peer->callno = find_callno(0, 0, &peer->addr, NEW_FORCE, peer->sockfd, 0);
12325
	callno = peer->callno = find_callno(0, 0, &peer->addr, NEW_FORCE, peer->sockfd, 0);
12326
	if (heldcall)
12326
	if (heldcall)
12327
		ast_mutex_lock(&iaxsl[heldcall]);
12327
		ast_mutex_lock(&iaxsl[heldcall]);
12328
	if (peer->callno < 1) {
12328
	if (callno < 1) {
12329
		ast_log(LOG_WARNING, "Unable to allocate call for poking peer '%s'\n", peer->name);
12329
		ast_log(LOG_WARNING, "Unable to allocate call for poking peer '%s'\n", peer->name);
12330
		return -1;
12330
		return -1;
12331
	}
12331
	}
12332

    
   
12332

   
12333
	/* Speed up retransmission times for this qualify call */

   
12334
	iaxs[peer->callno]->pingtime = peer->maxms / 4 + 1;

   
12335
	iaxs[peer->callno]->peerpoke = peer;

   
12336

    
   

   
12337
	if (peer->pokeexpire > -1) {
12333
	if (peer->pokeexpire > -1) {
12338
		if (!AST_SCHED_DEL(sched, peer->pokeexpire)) {
12334
		if (!AST_SCHED_DEL(sched, peer->pokeexpire)) {
12339
			peer->pokeexpire = -1;
12335
			peer->pokeexpire = -1;
12340
			peer_unref(peer);
12336
			peer_unref(peer);
12341
		}
12337
		}
[+20] [20] 10 lines
[+20] static int iax2_poke_peer(struct iax2_peer *peer, int heldcall)
12352
		peer_unref(peer);
12348
		peer_unref(peer);
12353

    
   
12349

   
12354
	/* And send the poke */
12350
	/* And send the poke */
12355
	ast_mutex_lock(&iaxsl[callno]);
12351
	ast_mutex_lock(&iaxsl[callno]);
12356
	if (iaxs[callno]) {
12352
	if (iaxs[callno]) {

    
   
12353
		/* Speed up retransmission times for this qualify call */

    
   
12354
		iaxs[callno]->pingtime = peer->maxms / 4 + 1;

    
   
12355
		iaxs[callno]->peerpoke = peer;

    
   
12356

   
12357
		struct iax_ie_data ied = {
12357
		struct iax_ie_data ied = {
12358
			.buf = { 0 },
12358
			.buf = { 0 },
12359
			.pos = 0,
12359
			.pos = 0,
12360
		};
12360
		};
12361
		add_empty_calltoken_ie(iaxs[callno], &ied); /* this _MUST_ be the last ie added */
12361
		add_empty_calltoken_ie(iaxs[callno], &ied); /* this _MUST_ be the last ie added */
[+20] [20] 2773 lines
  1. http://svn.asterisk.org/svn/asterisk/trunk/channels/chan_iax2.c: Loading...

https://reviewboard.asterisk.org/ runs on a server provided by Digium, Inc. and uses bandwidth donated to the open source Asterisk community by API Digital Communications in Huntsville, AL USA.
Please report problems with this site to asteriskteam@digium.com.