Commit Graph

8 Commits

Author SHA1 Message Date
Richard Mudgett c87e7dd9ec 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
2017-03-15 17:18:55 -06:00
Corey Farrell a6e5bae3ef Remove ASTERISK_REGISTER_FILE.
ASTERISK_REGISTER_FILE no longer has any purpose so this commit removes
all traces of it.

Previously exported symbols removed:
* __ast_register_file
* __ast_unregister_file
* ast_complete_source_filename

This also removes the mtx_prof static variable that was declared when
MTX_PROFILE was enabled.  This variable was only used in lock.c so it
is now initialized in that file only.

ASTERISK-26480 #close

Change-Id: I1074af07d71f9e159c48ef36631aa432c86f9966
2016-10-27 09:53:55 -04:00
Walter Doekes dcb25bb057 app_chanspy: Fix occasional deadlock with ChanSpy and Local channels.
Channel masquerading had a conflict with autochannel locking.

When locking autochannel->channel, the channel is fetched from the
autochannel and then locked. During the fetch, the autochannel -- which
has no locks itself -- can be modified by someone who owns the channel
lock. That means that the value of autochan->channel cannot be trusted
until you hold the lock.

In practice, this caused problems with Local channels getting
masqueraded away while the ChanSpy attempted to get info from that
channel. The old channel which was about to get removed got locked, but
the new (replaced) channel got unlocked (no-op). Because the replaced
channel was now locked (and would never get unlocked), it couldn't get
removed from the channel list in a timely manner, and would now cause
deadlocks when iterating over the channel list.

This change checks the autochannel after locking the channel for changes
to the autochannel. If the channel had been changed, the lock is
reobtained on the new channel.

In theory it seems possible that after this fix, the lock attempt on the
old (wrong) channel can be on an already destroyed lock, maybe causing
a crash. But that hasn't been observed in the wild and is harder induce
than the current deadlock.

Thanks go to Filip Frank for suggesting a fix similar to this and
especially to IRC user hexanol for pointing out why this deadlock was
possible and testing this fix. And to Richard for catching my rookie
while loop mistake ;)

ASTERISK-25321 #close

Change-Id: I293ae0014e531cd0e675c3f02d1d118a98683def
2016-03-11 16:05:30 -06:00
Matt Jordan 4a58261694 git migration: Refactor the ASTERISK_FILE_VERSION macro
Git does not support the ability to replace a token with a version
string during check-in. While it does have support for replacing a
token on clone, this is somewhat sub-optimal: the token is replaced
with the object hash, which is not particularly easy for human
consumption. What's more, in practice, the source file version was often
not terribly useful. Generally, when triaging bugs, the overall version
of Asterisk is far more useful than an individual SVN version of a file. As a
result, this patch removes Asterisk's support for showing source file
versions.

Specifically, it does the following:

* Rename ASTERISK_FILE_VERSION macro to ASTERISK_REGISTER_FILE, and
  remove passing the version in with the macro. Other facilities
  than 'core show file version' make use of the file names, such as
  setting a debug level only on a specific file. As such, the act of
  registering source files with the Asterisk core still has use. The
  macro rename now reflects the new macro purpose.

* main/asterisk:
  - Refactor the file_version structure to reflect that it no longer
    tracks a version field.
  - Remove the "core show file version" CLI command. Without the file
    version, it is no longer useful.
  - Remove the ast_file_version_find function. The file version is no
    longer tracked.
  - Rename ast_register_file_version/ast_unregister_file_version to
    ast_register_file/ast_unregister_file, respectively.

* main/manager: Remove value from the Version key of the ModuleCheck
  Action. The actual key itself has not been removed, as doing so would
  absolutely constitute a backwards incompatible change. However, since
  the file version is no longer tracked, there is no need to attempt to
  include it in the Version key.

* UPGRADE: Add notes for:
  - Modification to the ModuleCheck AMI Action
  - Removal of the "core show file version" CLI command

Change-Id: I6cf0ff280e1668bf4957dc21f32a5ff43444a40e
2015-04-13 03:48:57 -04:00
Kevin P. Fleming 166b4e2b30 Multiple revisions 369001-369002
........
  r369001 | kpfleming | 2012-06-15 10:56:08 -0500 (Fri, 15 Jun 2012) | 11 lines
  
  Add support-level indications to many more source files.
  
  Since we now have tools that scan through the source tree looking for files
  with specific support levels, we need to ensure that every file that is
  a component of a 'core' or 'extended' module (or the main Asterisk binary)
  is explicitly marked with its support level. This patch adds support-level
  indications to many more source files in tree, but avoids adding them to
  third-party libraries that are included in the tree and to source files
  that don't end up involved in Asterisk itself.
........
  r369002 | kpfleming | 2012-06-15 10:57:14 -0500 (Fri, 15 Jun 2012) | 3 lines
  
  Add a script to enable finding source files without support-levels defined.
........

Merged revisions 369001-369002 from http://svn.asterisk.org/svn/asterisk/branches/1.8
........

Merged revisions 369005 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@369013 65c4cc65-6c06-0410-ace0-fbb531ad65f3
2012-06-15 16:20:16 +00:00
Terry Wilson a9d607a357 Opaquify ast_channel structs and lists
Review: https://reviewboard.asterisk.org/r/1773/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@357542 65c4cc65-6c06-0410-ace0-fbb531ad65f3
2012-02-29 16:52:47 +00:00
Terry Wilson 04da92c379 Replace direct access to channel name with accessor functions
There are many benefits to making the ast_channel an opaque handle, from
increasing maintainability to presenting ways to kill masquerades. This patch
kicks things off by taking things a field at a time, renaming the field to
'__do_not_use_${fieldname}' and then writing setters/getters and converting the
existing code to using them. When all fields are done, we can move ast_channel
to a C file from channel.h and lop off the '__do_not_use_'.

This patch sets up main/channel_interal_api.c to be the only file that actually
accesses the ast_channel's fields directly. The intent would be for any API
functions in channel.c to use the accessor functions. No more monkeying around
with channel internals. We should use our own APIs.

The interesting changes in this patch are the addition of
channel_internal_api.c, the moving of the AST_DATA stuff from channel.c to
channel_internal_api.c (note: the AST_DATA stuff will have to be reworked to
use accessor functions when ast_channel is really opaque), and some re-working
of the way channel iterators/callbacks are handled so as to avoid creating fake
ast_channels on the stack to pass in matching data by directly accessing fields
(since "name" is a stringfield and the fake channel doesn't init the
stringfields, you can't use the ast_channel_name_set() function). I went with
ast_channel_name(chan) for a getter, and ast_channel_name_set(chan, name) for a
setter.

The majority of the grunt-work for this change was done by writing a semantic
patch using Coccinelle ( http://coccinelle.lip6.fr/ ).

Review: https://reviewboard.asterisk.org/r/1655/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@350223 65c4cc65-6c06-0410-ace0-fbb531ad65f3
2012-01-09 22:15:50 +00:00
Russell Bryant cba19c8a67 Convert the ast_channel data structure over to the astobj2 framework.
There is a lot that could be said about this, but the patch is a big 
improvement for performance, stability, code maintainability, 
and ease of future code development.

The channel list is no longer an unsorted linked list.  The main container 
for channels is an astobj2 hash table.  All of the code related to searching 
for channels or iterating active channels has been rewritten.  Let n be 
the number of active channels.  Iterating the channel list has gone from 
O(n^2) to O(n).  Searching for a channel by name went from O(n) to O(1).  
Searching for a channel by extension is still O(n), but uses a new method 
for doing so, which is more efficient.

The ast_channel object is now a reference counted object.  The benefits 
here are plentiful.  Some benefits directly related to issues in the 
previous code include:

1) When threads other than the channel thread owning a channel wanted 
   access to a channel, it had to hold the lock on it to ensure that it didn't 
   go away.  This is no longer a requirement.  Holding a reference is 
   sufficient.

2) There are places that now require less dealing with channel locks.

3) There are places where channel locks are held for much shorter periods 
   of time.

4) There are places where dealing with more than one channel at a time becomes 
   _MUCH_ easier.  ChanSpy is a great example of this.  Writing code in the 
   future that deals with multiple channels will be much easier.

Some additional information regarding channel locking and reference count 
handling can be found in channel.h, where a new section has been added that 
discusses some of the rules associated with it.

Mark Michelson also assisted with the development of this patch.  He did the 
conversion of ChanSpy and introduced a new API, ast_autochan, which makes it 
much easier to deal with holding on to a channel pointer for an extended period 
of time and having it get automatically updated if the channel gets masqueraded.
Mark was also a huge help in the code review process.

Thanks to David Vossel for his assistance with this branch, as well.  David 
did the conversion of the DAHDIScan application by making it become a wrapper 
for ChanSpy internally.

The changes come from the svn/asterisk/team/russell/ast_channel_ao2 branch.

Review: http://reviewboard.digium.com/r/203/


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@190423 65c4cc65-6c06-0410-ace0-fbb531ad65f3
2009-04-24 14:04:26 +00:00