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>

Diff revision 1

This is not the most recent revision of the diff. The latest diff is revision 2. See what's changed.

1 2
1 2

  1. trunk/main/channel.c: Loading...
trunk/main/channel.c
Revision 287464 New Change
[20] 5563 lines
[+20] [+] retrymasq:
5564
		return -1;
5564
		return -1;
5565
	}
5565
	}
5566

    
   
5566

   
5567
	ast_debug(1, "Planning to masquerade channel %s into the structure of %s\n",
5567
	ast_debug(1, "Planning to masquerade channel %s into the structure of %s\n",
5568
		clonechan->name, original->name);
5568
		clonechan->name, original->name);
5569
	if (original->masq) {
5569

   
5570
		ast_log(LOG_WARNING, "%s is already going to masquerade as %s\n",
5570
	if (!original->masqr && !original->masq && !clonechan->masq && !clonechan->masqr) {
5571
			original->masq->name, original->name);

   
5572
	} else if (clonechan->masqr) {

   
5573
		ast_log(LOG_WARNING, "%s is already going to masquerade as %s\n",

   
5574
			clonechan->name, clonechan->masqr->name);

   
5575
	} else {

   
5576
		original->masq = clonechan;
5571
		original->masq = clonechan;
5577
		clonechan->masqr = original;
5572
		clonechan->masqr = original;
5578
		ast_queue_frame(original, &ast_null_frame);
5573
		ast_queue_frame(original, &ast_null_frame);
5579
		ast_queue_frame(clonechan, &ast_null_frame);
5574
		ast_queue_frame(clonechan, &ast_null_frame);
5580
		ast_debug(1, "Done planning to masquerade channel %s into the structure of %s\n", clonechan->name, original->name);
5575
		ast_debug(1, "Done planning to masquerade channel %s into the structure of %s\n", clonechan->name, original->name);
5581
		res = 0;
5576
		res = 0;

    
   
5577
	} else if (original->masqr) {

    
   
5578
		/* not yet as a previously planned masq hasn't yet happened */

    
   
5579
		ast_log(LOG_WARNING, "%s (1) is already going to masquerade as %s\n",

    
   
5580
			original->name, original->masqr->name);

    
   
5581
	} else if (original->masq) {

    
   
5582
		ast_log(LOG_WARNING, "%s (2) is already going to masquerade as %s\n",

    
   
5583
			original->masq->name, original->name);

    
   
5584
	} else if (clonechan->masq) {

    
   
5585
		ast_log(LOG_WARNING, "%s (3) is already going to masquerade as %s\n",

    
   
5586
			clonechan->masq->name, clonechan->name);

    
   
5587
	} else { /* (clonechan->masqr) */

    
   
5588
		ast_log(LOG_WARNING, "%s (4) is already going to masquerade as %s\n",

    
   
5589
			clonechan->name, clonechan->masqr->name);
5582
	}
5590
	}
5583

    
   
5591

   
5584
	ast_channel_unlock(clonechan);
5592
	ast_channel_unlock(clonechan);
5585
	ast_channel_unlock(original);
5593
	ast_channel_unlock(original);
5586

    
   
5594

   
[+20] [20] 288 lines
[+20] [+] int ast_do_masquerade(struct ast_channel *original)
5875
		struct ast_party_redirecting redirecting;
5883
		struct ast_party_redirecting redirecting;
5876
	} exchange;
5884
	} exchange;
5877
	struct ast_channel *clonechan, *chans[2];
5885
	struct ast_channel *clonechan, *chans[2];
5878
	struct ast_channel *bridged;
5886
	struct ast_channel *bridged;
5879
	struct ast_cdr *cdr;
5887
	struct ast_cdr *cdr;
5880
	format_t rformat = original->readformat;
5888
	format_t rformat;
5881
	format_t wformat = original->writeformat;
5889
	format_t wformat;
5882
	char newn[AST_CHANNEL_NAME];
5890
	char newn[AST_CHANNEL_NAME];
5883
	char orig[AST_CHANNEL_NAME];
5891
	char orig[AST_CHANNEL_NAME];
5884
	char masqn[AST_CHANNEL_NAME];
5892
	char masqn[AST_CHANNEL_NAME];
5885
	char zombn[AST_CHANNEL_NAME];
5893
	char zombn[AST_CHANNEL_NAME];
5886

    
   
5894

   
[+20] [20] 36 lines
[+20] int ast_do_masquerade(struct ast_channel *original)
5923
	 * until we unlock the container. */
5931
	 * until we unlock the container. */
5924
	while (ast_channel_trylock(clonechan)) {
5932
	while (ast_channel_trylock(clonechan)) {
5925
		CHANNEL_DEADLOCK_AVOIDANCE(original);
5933
		CHANNEL_DEADLOCK_AVOIDANCE(original);
5926
	}
5934
	}
5927

    
   
5935

   

    
   
5936
	/* remember the read and write formats */

    
   
5937
	rformat = original->readformat;

    
   
5938
	wformat = original->writeformat;

    
   
5939

   
5928
	/* clear the masquerade channels */
5940
	/* clear the masquerade channels */
5929
	original->masq = NULL;
5941
	original->masq = NULL;
5930
	clonechan->masqr = NULL;
5942
	clonechan->masqr = NULL;
5931

    
   
5943

   
5932
	/* 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 */
5933
	ao2_unlink(channels, original);
5945
	ao2_unlink(channels, original);
5934
	ao2_unlink(channels, clonechan);
5946
	ao2_unlink(channels, clonechan);
5935

    
   
5947

   
5936
	/* 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
	 */
5937
	ao2_unlock(channels);
5953
	ao2_unlock(channels);
5938

    
   
5954
#endif
5939
	ast_debug(4, "Actually Masquerading %s(%d) into the structure of %s(%d)\n",
5955
	ast_debug(4, "Actually Masquerading %s(%d) into the structure of %s(%d)\n",
5940
		clonechan->name, clonechan->_state, original->name, original->_state);
5956
		clonechan->name, clonechan->_state, original->name, original->_state);
5941

    
   
5957

   
5942
	chans[0] = clonechan;
5958
	chans[0] = clonechan;
5943
	chans[1] = original;
5959
	chans[1] = original;
[+20] [20] 271 lines
[+20] [+] done:
6215
		ao2_link(channels, original);
6231
		ao2_link(channels, original);
6216
	} else {
6232
	} else {
6217
		ast_channel_unlock(original);
6233
		ast_channel_unlock(original);
6218
		ao2_link(channels, original);
6234
		ao2_link(channels, original);
6219
	}
6235
	}
6220

    
   
6236
	/* unlock channels here to prevent 'Bad Magic Number' */

    
   
6237
	ao2_unlock(channels);
6221
	return 0;
6238
	return 0;
6222
}
6239
}
6223

    
   
6240

   
6224
void ast_set_callerid(struct ast_channel *chan, const char *cid_num, const char *cid_name, const char *cid_ani)
6241
void ast_set_callerid(struct ast_channel *chan, const char *cid_num, const char *cid_name, const char *cid_ani)
6225
{
6242
{
[+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.