diff options
| author | Richard Mudgett <rmudgett@digium.com> | 2017-03-15 13:24:33 -0500 |
|---|---|---|
| committer | Richard Mudgett <rmudgett@digium.com> | 2017-03-15 17:18:55 -0600 |
| commit | c87e7dd9ec01b0e0cfcbd3a2c2207b924201813e (patch) | |
| tree | 8def1f9eb70a26a1d3eb09fd7f14c36fb135f847 /include/asterisk/autochan.h | |
| parent | 3fe1d8afbaf95bacd19d6a02f6ebdf9e5a238f53 (diff) | |
autochan/mixmonitor/chanspy: Fix unsafe channel locking and references.
Dereferencing struct ast_autochan.chan without first calling
ast_autochan_channel_lock() is unsafe because the pointer could change at
any time due to a masquerade. Unfortunately, ast_autochan_channel_lock()
itself uses struct ast_autochan.chan unsafely and can result in a deadlock
if the original channel happens to get destroyed after a masquerade in
addition to the pointer getting changed.
The problem is more likely to happen with v11 and earlier because
masquerades are used to optimize out local channels on those versions.
However, it could still happen on newer versions if the channel is
executing a dialplan application when the channel is transferred or
redirected. In this situation a masquerade still must be used.
* Added a lock to struct ast_autochan to safely be able to use
ast_autochan.chan while trying to get the channel lock in
ast_autochan_channel_lock(). The locking order is the channel lock then
the autochan lock. Locking in the other direction requires deadlock
avoidance.
* Fix unsafe ast_autochan.chan usages in app_mixmonitor.c.
* Fix unsafe ast_autochan.chan usages in app_chanspy.c.
* app_chanspy.c: Removed unused autochan parameter from next_channel().
ASTERISK-26867
Change-Id: Id29dd22bc0f369b44e23ca423d2f3657187cc592
Diffstat (limited to 'include/asterisk/autochan.h')
| -rw-r--r-- | include/asterisk/autochan.h | 20 |
1 files changed, 13 insertions, 7 deletions
diff --git a/include/asterisk/autochan.h b/include/asterisk/autochan.h index 319c203ab..128377b57 100644 --- a/include/asterisk/autochan.h +++ b/include/asterisk/autochan.h @@ -32,6 +32,7 @@ struct ast_autochan { struct ast_channel *chan; AST_LIST_ENTRY(ast_autochan) list; + ast_mutex_t lock; }; /*! @@ -61,19 +62,24 @@ struct ast_autochan { * ast_autochan_channel_lock and ast_autochan_channel_unlock. An attempt to lock * the autochan->chan directly may result in it being changed after you've * retrieved the value of chan, but before you've had a chance to lock it. - * First when chan is locked, the autochan structure is guaranteed to keep the + * While chan is locked, the autochan structure is guaranteed to keep the * same channel. */ +/*! + * \brief Lock the autochan's channel lock. + * + * \note We must do deadlock avoidance because the channel lock is + * superior to the autochan lock in locking order. + */ #define ast_autochan_channel_lock(autochan) \ do { \ - struct ast_channel *autochan_chan = autochan->chan; \ - ast_channel_lock(autochan_chan); \ - if (autochan->chan == autochan_chan) { \ - break; \ + ast_mutex_lock(&(autochan)->lock); \ + while (ast_channel_trylock((autochan)->chan)) { \ + DEADLOCK_AVOIDANCE(&(autochan)->lock); \ } \ - ast_channel_unlock(autochan_chan); \ - } while (1) + ast_mutex_unlock(&(autochan)->lock); \ + } while (0) #define ast_autochan_channel_unlock(autochan) \ ast_channel_unlock(autochan->chan) |