<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3135/">https://reviewboard.asterisk.org/r/3135/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#e0e0e0" width="100%" cellpadding="8" style="border: 1px gray solid;">
<tr>
<td>
<h1 style="margin-right: 0.2em; padding: 0; font-size: 10pt;">This change has been marked as submitted.</h1>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers and leifmadsen.</div>
<div>By Russell Bryant.</div>
<p style="color: grey;"><i>Updated Jan. 27, 2014, 1:25 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">The ast_filestream object gets tacked on to a channel via
chan->timingdata. It's a reference counted object, but the reference
count isn't used when putting it on a channel. It's theoretically
possible for another thread to interfere with the channel while it's
unlocked and cause the filestream to get destroyed.
Use the astobj2 reference count to make sure that as long as this code
path is holding on the ast_filestream and passing it into the file.c
playback code, that it knows it's valid.
-----
More detail:
A crash occurs in voicemail. Here is the backtrace:
#0 0x00000000004d396a in ast_readaudio_callback (s=0x7f260856f580) at file.c:779
#1 0x00000000004d3b8a in ast_fsread_audio (data=0x7f260856f580) at file.c:806
#2 0x000000000047599d in __ast_read (chan=0x7f2644c49460, dropaudio=0) at channel.c:3865
#3 0x0000000000477d81 in ast_read (chan=0x7f2644c49460) at channel.c:4325
#4 0x00000000004d5231 in waitstream_core (c=0x7f2644c49460, breakon=0x7f262ed56f00 "#*", forward=0x64c4a2 "", reverse=0x64c4a2 "", skip_ms=0, audiofd=-1, cmdfd=-1, context=0x0) at file.c:1253
#5 0x00000000004d5783 in ast_waitstream (c=0x7f2644c49460, breakon=0x7f262ed56f00 "#*") at file.c:1344
#6 0x00007f266d34c22b in leave_voicemail (chan=0x7f2644c49460, ext=0x7f2609908c20 "<redacted>", options=0x7f262ed56ff0) at app_voicemail.c:5773
#7 0x00007f266d35eb71 in vm_exec (chan=0x7f2644c49460, data=0x7f262ed59720 "<redacted>") at app_voicemail.c:10722
(gdb) frame 0
#0 0x00000000004d396a in ast_readaudio_callback (s=0x7f260856f580) at file.c:779
779 if (s->owner->timingfd > -1) {
(gdb) p s
$6 = (struct ast_filestream *) 0x7f260856f580
The crash occurs here because the contents of the ast_filestream are garbage.
(gdb) p *s
$7 = {fmt = 0x6372756f530a0d74, flags = 924858981, mode = 875902769, open_filename = 0x440a0d3330393734 <Address 0x440a0d3330393734 out of bounds>, filename = 0x6974616e69747365 <Address 0x6974616e69747365 out of bounds>,
realfilename = 0x440a0d73203a6e6f <Address 0x440a0d73203a6e6f out of bounds>, vfs = 0x6974616e69747365, trans = 0x7865746e6f436e6f, tr = 0x2d7473786e203a74, lastwriteformat = 762475363, lasttimeout = 1969583473,
owner = 0x656c6c61430a0d65, f = 0x313722203a444972, fr = {frametype = 875836727, subclass = {integer = 926687266, codec = 3761973748257529890}, datalen = 809056052, samples = 168640051, mallocd = 1851877443,
mallocd_hdr_len = 7881689877736674080, offset = 762148397, src = 0xd61633832313030 <Address 0xd61633832313030 out of bounds>, data = {ptr = 0x616e69747365440a, uint32 = 1936016394, pad = "\nDestina"}, delivery = {
tv_sec = 7953753055737899380, tv_usec = 5785246594218354030}, frame_list = {next = 0x36702d5453584e2f}, flags = 758134073, ts = 7010989772577650738, len = 7163375912484959347, seqno = 1869182049},
buf = 0x614c0a0d65756575 <Address 0x614c0a0d65756575 out of bounds>, _private = 0x203a617461447473, orig_chan_name = 0x7273632d7473786e <Address 0x7273632d7473786e out of bounds>,
write_buffer = 0x38312c2c2c2c712d <Address 0x38312c2c2c2c712d out of bounds>}
(gdb) p s->owner
$8 = (struct ast_channel *) 0x656c6c61430a0d65
(gdb) p *s->owner
Cannot access memory at address 0x656c6c61430a0d65
s->owner should have been 0x7f2644c49460, from higher up in the backtrace.
Here is where things get quite interesting ...
(gdb) frame 2
#2 0x000000000047599d in __ast_read (chan=0x7f2644c49460, dropaudio=0) at channel.c:3865
3865 func(data);
(gdb) list
3860 /* save a copy of func/data before unlocking the channel */
3861 int (*func)(const void *) = chan->timingfunc;
3862 void *data = chan->timingdata;
3863 chan->fdno = -1;
3864 ast_channel_unlock(chan);
3865 func(data);
3866 } else {
3867 ast_timer_set_rate(chan->timer, 0);
3868 chan->fdno = -1;
3869 ast_channel_unlock(chan);
(gdb) p chan->timingfunc
$9 = (int (*)(const void *)) 0
(gdb) p chan->timingdata
$10 = (void *) 0x0
(gdb) p func
$11 = (int (*)(const void *)) 0x4d3b6a <ast_fsread_audio>
(gdb) p data
$12 = (void *) 0x7f260856f580
This is the code inside of ast_read() that executes the callback ast_fsread_audio() from main/file.c to play the next bits of the audio file to the channel. We can see that chan->timingfunc and chan->timingdata have now been reset since this code ran. That probably also means that the ast_filestream got destroyed before the code in ast_readaudio_callback was done using it.
The only way I can think of this happening is via something in another thread. Something like AMI running on a channel doing something that affects audio. I'm basically speculating at this point. If this is the case, it seems like the filestream's reference count needs to be bumped while it's on the channel. Otherwise, it seems possible that it could disappear at just the wrong time.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This issue was originally reported by Leif Madsen. The patch was given to him for further testing. They have done some basic sanity tests with it, but it's not yet running in production or anything like that. It at least doesn't blow up immediately ...
I wanted to get the patch and analysis up to get some more eyes on it.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/1.8/main/file.c <span style="color: grey">(405876)</span></li>
<li>/branches/1.8/main/channel.c <span style="color: grey">(405876)</span></li>
<li>/branches/1.8/include/asterisk/channel.h <span style="color: grey">(405876)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3135/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>