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 2 (Latest)

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 */
[+20] [20] 272 lines
[+20] int ast_do_masquerade(struct ast_channel *original)
6205
	}
6217
	}
6206

    
   
6218

   
6207
	ast_indicate(original, AST_CONTROL_SRCCHANGE);
6219
	ast_indicate(original, AST_CONTROL_SRCCHANGE);
6208

    
   
6220

   
6209
done:
6221
done:
6210
	/* 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
	 */
6211
	if (clonechan) {
6225
	if (clonechan) {
6212
		ast_channel_unlock(original);

   
6213
		ast_channel_unlock(clonechan);

   
6214
		ao2_link(channels, clonechan);
6226
		ao2_link(channels, clonechan);
6215
		ao2_link(channels, original);
6227
		ao2_link(channels, original);
6216
	} else {
6228
		ast_channel_unlock(clonechan);
6217
		ast_channel_unlock(original);
6229
		ast_channel_unlock(original);

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

    
   
6232
		ast_channel_unlock(original);
6219
	}
6233
	}
6220

    
   
6234

   
6221
	return 0;
6235
	return 0;
6222
}
6236
}
6223

    
   
6237

   
[+20] [20] 2690 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.