Review Board 1.7.16


Prevent 'Bad Magic Number' caused when a channel is optimized out by masquerade

Review Request #928 - Created Sept. 17, 2010 and submitted

Alec Davis
trunk
16057, 17037, 17363, 17801
Reviewers
asterisk-dev
Asterisk
there are 2 main fixes here.

1). Prevent a further masquerade being planned if either original/clonechan(masq/masqr) are set.
2). While the actual masquerade happens keep the channels container locked for the full duration. 

There is a comment 'that the channels container can be freed' after both channels are locked and unlink, but that not the case.
Using the following test plan, calling 10020 creates 20 looped Local channel calls, that then get optimized out.

[test]
exten => 10000,1,Answer()
exten => 10000,n,Playback(test-tones-follow)
exten => 10000,n,Milliwatt()

exten => _1XXXX,1,Set(i=${MATH(${EXTEN}-1,int)})
exten => _1XXXX,n,Dial(Local/${i}@test)

After patch: normal expected channels.
    asterix*CLI> core show channels
    Channel              Location             State   Application(Data)
    DAHDI/35-1           10010@phones:2       Up      Dial(Local/10009@phones)
    Local/10000@phones-2 s@echo-test:4        Up      Echo()
    Local/10000@phones-2 (None)               Up      AppDial((Outgoing Line))
    3 active channels
    2 active calls
    97 calls processed
    asterix*CLI>


prior to the patch:
   'Bad Magic Number' would reqularly been seen.

   dead channels left in container
   asterix*CLI> core show channels
   Channel              Location             State   Application(Data)
   Local/10000@phones-2 s@echo-test:3        Up      Playback(echo-test)
   Local/10000@phones-2 (None)               Up      AppDial((Outgoing Line))
   2 active channels
   1 active call
   21 calls processed
   asterix*CLI>

Changes between revision 1 and 2

1 2
1 2

  1. trunk/main/channel.c: Loading...
trunk/main/channel.c
Diff Revision 1 Diff Revision 2
[20] 5943 lines
[+20] [+] int ast_do_masquerade(struct ast_channel *original)
5944
	/* unlink from channels container as name (which is the hash value) will change */
5944
	/* unlink from channels container as name (which is the hash value) will change */
5945
	ao2_unlink(channels, original);
5945
	ao2_unlink(channels, original);
5946
	ao2_unlink(channels, clonechan);
5946
	ao2_unlink(channels, clonechan);
5947

    
   
5947

   
5948
	/* now that both channels are locked and unlinked from the container, it is safe to unlock it */
5948
	/* now that both channels are locked and unlinked from the container, it is safe to unlock it */
5949
#if 0

   
5950
	/* don't unlock channels container here, as this seems to cause 'Bad magic number'

   
5951
	 * unlock currently deferred to end of routine

   
5952
	 */

   
5953
	ao2_unlock(channels);
5949
	ao2_unlock(channels);
5954
#endif
5950

   
5955
	ast_debug(4, "Actually Masquerading %s(%d) into the structure of %s(%d)\n",
5951
	ast_debug(4, "Actually Masquerading %s(%d) into the structure of %s(%d)\n",
5956
		clonechan->name, clonechan->_state, original->name, original->_state);
5952
		clonechan->name, clonechan->_state, original->name, original->_state);
5957

    
   
5953

   
5958
	chans[0] = clonechan;
5954
	chans[0] = clonechan;
5959
	chans[1] = original;
5955
	chans[1] = original;
[+20] [20] 261 lines
[+20] int ast_do_masquerade(struct ast_channel *original)
6221
	}
6217
	}
6222

    
   
6218

   
6223
	ast_indicate(original, AST_CONTROL_SRCCHANGE);
6219
	ast_indicate(original, AST_CONTROL_SRCCHANGE);
6224

    
   
6220

   
6225
done:
6221
done:
6226
	/* it is possible for the clone channel to disappear during this */
6222
	/* link each channel into the channels container before unlocking each channel,

    
   
6223
	 * otherwise the channel may disappear, and cause 'Bad magic number'

    
   
6224
	 */
6227
	if (clonechan) {
6225
	if (clonechan) {
6228
		ast_channel_unlock(original);

   
6229
		ast_channel_unlock(clonechan);

   
6230
		ao2_link(channels, clonechan);
6226
		ao2_link(channels, clonechan);
6231
		ao2_link(channels, original);
6227
		ao2_link(channels, original);
6232
	} else {
6228
		ast_channel_unlock(clonechan);
6233
		ast_channel_unlock(original);
6229
		ast_channel_unlock(original);

    
   
6230
	} else {
6234
		ao2_link(channels, original);
6231
		ao2_link(channels, original);

    
   
6232
		ast_channel_unlock(original);
6235
	}
6233
	}
6236
	/* unlock channels here to prevent 'Bad Magic Number' */
6234

   
6237
	ao2_unlock(channels);

   
6238
	return 0;
6235
	return 0;
6239
}
6236
}
6240

    
   
6237

   
6241
void ast_set_callerid(struct ast_channel *chan, const char *cid_num, const char *cid_name, const char *cid_ani)
6238
void ast_set_callerid(struct ast_channel *chan, const char *cid_num, const char *cid_name, const char *cid_ani)
6242
{
6239
{
[+20] [20] 2688 lines
  1. trunk/main/channel.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.