Summary:ASTERISK-19550: Segfault in ast_readaudio_callback
Reporter:Steve Davies (one47)Labels:
Date Opened:2012-03-15 17:12:42Date Closed:
Versions:1.4.43 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 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.