| Summary: | ASTERISK-19550: Segfault in ast_readaudio_callback | ||
| Reporter: | Steve Davies (one47) | Labels: | |
| Date Opened: | 2012-03-15 17:12:42 | Date Closed: | |
| Priority: | Major | Regression? | No |
| Status: | Open/New | Components: | Core/FileFormatInterface |
| Versions: | 1.4.43 1.6.2.22 1.8.10.0 1.8.22.0 10.2.0 10.12.2 11.4.0 13.18.4 | Frequency of Occurrence | Occasional |
| Related Issues: | |||
| 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
Cause: - 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. Under-the-hood: 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 1.6.2.22. 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. | ||