|Summary:||ASTERISK-19550: Segfault in ast_readaudio_callback|
|Reporter:||Steve Davies (one47)||Labels:|
|Date Opened:||2012-03-15 17:12:42||Date Closed:|
|Versions:||1.4.43 188.8.131.52 184.108.40.206 220.127.116.11 10.2.0 10.12.2 11.4.0 13.18.4||Frequency of|
|Environment:||Easier to reproduce in Intel multi-core servers.||Attachments:||( 0) readaudio_callback_workaround|
|Description:||Issue believed to occur on all versions. Raised here after email from Mark Michelson on -dev mailing list
- SIP Refer (attended transfer) a caller into a Queue while the position announce is playing.
- The crash happens more easily on a fast multi-core server.
The Queue position announce is being played using ast_readaudio_callback, which is called with an ast_filestream*, usually every 20ms.
In the meantime, The SIP Refer sets up a masquerade from within a separate thread. If that masquerade is picked-up by ast_read() or ast_waitfor_nandfs(), then all is happy, BUT, the ast_readaudio_callback() function uses ast_write, which can also trigger the ast_do_masquerade().
Depending on thread scheduling, the call to ast_do_masquerade can cause the stream's channel to be freed, or sometimes just marked as a ZOMBIE, but either way, the ast_filestream is usually closed using ast_closestream(), setting it's ao2 refcount to zero, destroying it, and allowing the memory to be re-used.
At this point execution passes back from the ast_write() to ast_readaudio_callback() where it tries to use the freed ast_filestream, which in my testing, generally points at garbage data, and if it survives that experience, it tries to access the freed channel instead.
|Comments:||By: Steve Davies (one47) 2012-03-15 17:14:18.933-0500|
Bug hard to reproduce. Attaching fix which applies to 18.104.22.168. Comment from Mark Michelson pasted below which relates to porting the patch to versions 1.8/10:
>> 2) When calling ast_closestream(), set the stream->owner to NULL so it
>> is clear that the channel should no longer be used by that stream.
> This is probably the best way to go about this in the version you are using.
> However, in 1.8+, the better approach would be to make sure that when the
> filestream sets its owner, it bumps the refcount of the channel. This way,
> we won't need the extra if (s->owner) checks all over the place and we can
> safely refer to the channel throughout the life of the filestream. We just
> would need to be sure to decrement the refcount on the owner if either a)
> ast_applystream() is called with a new supplied channel or b) the filestream
> is destroyed.
By: Steve Davies (one47) 2013-05-21 11:56:56.067-0500
Has this issue been resolved in 11.4.0 ? Looking at the code, it has not, but perhaps I am missing it? Neither the patch for 1.6 above, nor the alternative suggested by Mark Michelson appears to have been applied.
Has it been fixed in some other way that I am missing?
By: Matt Jordan (mjordan) 2013-05-21 12:26:48.687-0500
Nope, it still hasn't been fixed.