Russell Bryant [Thu, 8 Oct 2009 19:45:47 +0000 (19:45 +0000)]
Make filestream frame handling safer by isolating frames before returning them.
This patch is related to a number of issues on the bug tracker that show
crashes related to freeing frames that came from a filestream. A number of
fixes have been made over time while trying to figure out these problems, but
there re still people seeing the crash. (Note that some of these bug reports
include information about other problems. I am specifically addressing
the filestream frame crash here.)
I'm still not clear on what the exact problem is. However, what is _very_
clear is that we have seen quite a few problems over time related to unexpected
behavior when we try to use embedded frames as an optimization. In some cases,
this optimization doesn't really provide much due to improvements made in other
areas.
In this case, the patch modifies filestream handling such that the embedded frame
will not be returned. ast_frisolate() is used to ensure that we end up with a
completely mallocd frame. In reality, though, we will not actually have to malloc
every time. For filestreams, the frame will almost always be allocated and freed
in the same thread. That means that the thread local frame cache will be used.
So, going this route doesn't hurt.
With this patch in place, some people have reported success in not seeing the
crash anymore.
(SWP-150)
(AST-208)
(ABE-1834)
(issue #15609)
Reported by: aragon
Patches:
filestream_frisolate-1.4.diff2.txt uploaded by russell (license 2)
Tested by: aragon, russell
Richard Mudgett [Thu, 8 Oct 2009 16:33:06 +0000 (16:33 +0000)]
Fix memory leak if chan_misdn config parameter is repeated.
Memory leak when the same config option is set more than once in an
misdn.conf section. Why must this be considered? Templates! Defining a
template with default port options and later adding to or overriding some
of them.
Richard Mudgett [Wed, 7 Oct 2009 21:51:24 +0000 (21:51 +0000)]
chan_misdn.c:process_ast_dsp() memory leak
misdn.conf: astdtmf must be set to "yes". With "no", buffer loss does not
occur.
The translated frame "f2" when passing through ast_dsp_process() is not
freed whenever it is not used further in process_ast_dsp(). Then in the
end it is never ever freed.
Jeff Peeler [Tue, 6 Oct 2009 22:27:13 +0000 (22:27 +0000)]
Fix potential crash when entire span request is received.
The variable index used in this scenario for accessing the dahdi_pvts was
wrong and was most likely copied from the several other places it is used
correctly.
Fix ao2_iterator API to hold references to containers being iterated.
See Mantis issue for details of what prompted this change.
Additional notes:
This patch changes the ao2_iterator API in two ways: F_AO2I_DONTLOCK
has become an enum instead of a macro, with a name that fits our
naming policy; also, it is now necessary to call
ao2_iterator_destroy() on any iterator that has been
created. Currently this only releases the reference to the container
being iterated, but in the future this could also release other
resources used by the iterator, if the iterator implementation changes
to use additional resources.
Richard Mudgett [Thu, 1 Oct 2009 23:18:28 +0000 (23:18 +0000)]
Occasionally losing use of B channels in chan_misdn.
I have not been able to reproduce the problem of losing channels.
However, I have seen in the code a reentrancy problem that might give
these symptoms.
The reentrancy patch does several things:
1) Guards B channel and B channel structure allocation.
2) Makes the B channel structure find routines more precise in locating records.
3) Never leave a B channel allocated if we received cause 44.
The last item may cause temporary outgoing call problems, but they should
clear when the line becomes idle.
Fix SRV lookup and Request-URI generation in chan_sip.
This patch adds a new field "portinuri" to the sip dialog struct and the sip peer struct. That field is used during RURI generation to determine if the port should be included in the RURI. It is also used in some places to determine if an SRV lookup should occur.
Terry Wilson [Wed, 30 Sep 2009 14:49:11 +0000 (14:49 +0000)]
Change the SSRC by default when our media stream changes
Be default, change SSRC when doing an audio stream changes Asterisk doesn't
honor marker bit when reinvited to already-bridged RTP streams,resulting in
far-end stack discarding packets with "old" timestamps that areactually part of
a new stream. This patch sends AST_CONTROL_SRCUPDATE whenever there is a
reinvite, unless the 'constantssrc' is set to true in sip.conf.
The original issue reported to Digium support detailed the following situation:
ITSP <-> Asterisk 1.4.26.2 <-> SIP-based Application Server Call comes in
fromITSP, Asterisk dials the app server which sends a re-invite back
toAsterisk--not to negotiate to send media directly to the ITSP, but to
indicatethat it's changing the stream it's sending to Asterisk. The app
servergenerates a new SSRC, sequence numbers, timestamps, and sets the marker
bit on the new stream. Asterisk passes through the teimstamp of the new stream,
butdoes not reset the SSRC, sequence numbers, or set the marker bit.
When the timestamp on the new stream is older than the timestamp on the
originalstream, the ITSP (which doesn't know there has been any change) discards
the newframes because it thinks they are too old. This patch addresses this by
changing the SSRC on a stream update unless constantssrc=true is set in
sip.conf.
Reduce CPU usage related to building a peer merely for devicestates.
This fixes a 100% CPU problem in the SIP driver, found by profiling
the driver while the problem was occurring.
(closes issue #14309)
Reported by: pkempgen
Patches:
20090924__issue14309.diff.txt uploaded by tilghman (license 14)
Tested by: pkempgen, vrban
Sean Bright [Mon, 28 Sep 2009 19:09:25 +0000 (19:09 +0000)]
When selecting DONT_OPTIMIZE in menuselect, explicitly pass -O0 to the compiler
so we override any default optimization levels for a particular install.
Implicitly sending a progress signal breaks some applications.
Call Progress() in your dialplan if you explicitly want progress to be sent.
(Reverts change 216430, closes issue #15957)
Reported by: Pavel Troller on the Asterisk-Dev mailing list
http://lists.digium.com/pipermail/asterisk-dev/2009-September/039897.html
When IMAP variables were changed during a reload, Voicemail did not use the new values.
This change introduces a configuration version variable, which ensures that
connections with the old values are not reused but are allowed to expire
normally.
(closes issue #15934)
Reported by: viniciusfontes
Patches:
20090922__issue15934.diff.txt uploaded by tilghman (license 14)
Tested by: viniciusfontes
David Vossel [Fri, 18 Sep 2009 23:19:50 +0000 (23:19 +0000)]
iax2 frame double free
The iax frame's retrans sched id was written over right
before iax2_frame_free was called. In iax2_frame_free that
retrans id is used to delete the sched item. By writing over
the retrans field before the sched item could be deleted, it was
possible for a retransmit to occur on a freed frame.
David Vossel [Fri, 18 Sep 2009 16:19:15 +0000 (16:19 +0000)]
via-header branches not updated correctly on INVITE
INVITE requests must always contain a new unique branch id. When
a new branch id is created for an INVITE, the dialog's invite_branch
variable must be updated so CANCEL requests use the correct branch id.
David Vossel [Thu, 17 Sep 2009 21:29:37 +0000 (21:29 +0000)]
INVITE w/Replaces deadlock fix
This patch cleans up the locking logic in chan_sip.c's
handle_invite_replaces() function as well as making use
of ast_do_masquerade() rather than forcing the masquerade
on an ast_read(). The code had several redundant unlocks
that would result in 'freed more times than we've locked!'
errors. I cleaned these up as well as moving all the unlock
logic to the end of the function. This patch should also
resolve the issue people were having with the replacecall
channel never being unlocked with one legged calls.
Prevent a potential race condition and crash when hanging up a channel by removing the channel from the channel list before begining channel tear down.
This fix may potentially cause problems with CDR backends that access the channel a CDR is associated with via the channel list. This fix makes the channel unavabile at the time when the CDR backend is invoked. This has been documented in include/asterisk/cdr.h.
David Brooks [Wed, 16 Sep 2009 18:00:45 +0000 (18:00 +0000)]
Fixes CID pattern matching behavior to mirror that of extension pattern matching.
Pattern matching for extensions uses a type of scoring system, giving values for
specificity to each character in the pattern. Unfortunately, this is done character
by character, in order. This does lead to some less specific patterns being first
in line for matching, but it will usually get the job done.
This patch merely brings CID matching to the same level as extension matching.
This patch does not attempt to tackle the problem shared by extension matching.
Russell Bryant [Wed, 16 Sep 2009 13:33:43 +0000 (13:33 +0000)]
Remove the IAXy firmware from Asterisk.
The firmware can now be found on downloads.digium.com, where the rest of our
binary downloads live. This was the last part of our Asterisk tarballs that
was considered non-free by Debian. :-)
If the user enters the same password as before, don't signal an error when the change does nothing.
(closes issue #15492)
Reported by: cbbs70a
Patches:
20090713__issue15492.diff.txt uploaded by tilghman (license 14)
Ensure FollowMe sets language in channels it creates.
Also, not in the original bug report, but related fields are accountcode and
musicclass, and the inheritance of datastores.
(closes issue #15372)
Reported by: Romik
Patches:
20090828__issue15372.diff.txt uploaded by tilghman (license 14)
Tested by: cervajs
Jeff Peeler [Mon, 14 Sep 2009 21:47:11 +0000 (21:47 +0000)]
Fix handling of DAHDI_EVENT_REMOVED event to prevent crash in do_monitor.
After talking to rmudgett about some of his recent iflist locking changes, it
was determined that the only place that would destroy a channel without being
explicitly to do so was in handle_init_event. The loop to walk the interface
list has been modified to wait to destroy the channel until the dahdi_pvt of
the channel to be destroyed is no longer needed.
David Vossel [Thu, 10 Sep 2009 21:06:07 +0000 (21:06 +0000)]
IAX2 encryption regression
The IAX2 Call Token security patch inadvertently broke the use of
encryption due to the reorganization of code in the socket_process()
function. When encryption is used, an incoming full frame must first
be decrypted before the information elements can be parsed. The
security release mistakenly moved IE parsing before decryption in
order to process the new Call Token IE. To resolve this, decryption
of full frames is once again done before looking into the frame. This
involves searching for an existing callno, checking the pvt to see if
encryption is turned on, and decrypting the packet before the internal
fields of the full frame are accessed.
Olle Johansson [Thu, 10 Sep 2009 19:07:24 +0000 (19:07 +0000)]
Remove harmful code that causes endless loops.
Remove code that causes loops in registrations.
We have agreed that the patch that this code was part of was bad. I am ripping out the code that causes
the issue. putnopvut needs to check the rest of the patch, if it needs to be changed as well.
This solves the issue reported in #15540, but needs more work before we close it (as described above).
When MOH is playing on the channel, announcements sent through the conference are not heard.
(closes issue #14588)
Reported by: voipas
Patches:
20090716__issue14588__2.diff.txt uploaded by tilghman (license 14)
Tested by: lmadsen, twisted, tilghman
Olle Johansson [Fri, 4 Sep 2009 13:45:48 +0000 (13:45 +0000)]
Make apps send PROGRESS control frame for early media and fix too early media issue in SIP
The issue at hand is that some legacy (dying) PBX systems send empty media frames on PRI
links *before* any call progress. The SIP channel receives these frames and by default
signals 183 Session progress and starts sending media. This will cause phones to
play silence and ignore the later 180 ringing message. A bad user experience.
The fix is twofold:
- We discovered that asterisk apps that support early media ("noanswer") did not send
any PROGRESS frame to indicate early media. Fixed.
- We introduce a setting in chan_sip so that users can disable any relay of media frames
before the outbound channel actually indicates any sort of call progress.
In 1.4, 1.6.0 and 1.6.1, this will be disabled for backward compatibility. In later versions
of Asterisk, this will be enabled. We don't assume that it will change your Asterisk
phone experience - only for the better.
We encourage third-party application developers to make sure that if they have applications
that wants to send early media, add a PROGRESS control frame transmission to make sure that
all channel drivers actually will start sending early media. This has not been the default
in Asterisk previous to this patch, so if you got inspiration from our code, you need to
update accordingly. Sorry for the trouble and thanks for your support.
This code has been running for a few months in a large scale installation (over 250
servers with PRI and/or BRI links to old PBX systems).
That's no proof that this is an excellent patch, but, well, it's tested :-)
Terry Wilson [Wed, 2 Sep 2009 21:41:22 +0000 (21:41 +0000)]
Re-send non-100 provisional responses to prevent cancellation
From section 13.3.1.1 of RFC 3261:
If the UAS desires an extended period of time to answer the INVITE,
it will need to ask for an "extension" in order to prevent proxies
from canceling the transaction. A proxy has the option of canceling
a transaction when there is a gap of 3 minutes between responses in a
transaction. To prevent cancellation, the UAS MUST send a non-100
provisional response at every minute, to handle the possibility of
lost provisional responses.
Use strrchr() so SoftHangup will correctly truncate multi-hyphen channel names
In general channel names are in the form Foo/Bar-Z, but the channel name
could have multiple hyphens and look like Foo/B-a-r-Z. Use strrchr to
truncate the channel name at the last hyphen.
Tilghman Lesher [Mon, 31 Aug 2009 16:16:52 +0000 (16:16 +0000)]
Also unlock the "other" channel, when returning, due to glare.
(closes issue #15787)
Reported by: tim_ringenbach
Patches:
chan_local.diff uploaded by tim ringenbach (license 540)
Tested by: tim_ringenbach
Tilghman Lesher [Fri, 28 Aug 2009 20:13:32 +0000 (20:13 +0000)]
Modify comment to be a bit more accurate.
We have kept this comment around long enough, that it's pretty clear that we're
keeping the code, because changing the code would require a pretty fundamental
architectural shift. We've also taken criticism in some quarters, because it
was believed that it was referring to the code being nasty. No, the code isn't
nasty, just the operation itself is rather odd. Fixed for eternity (probably
not).
David Vossel [Wed, 26 Aug 2009 16:36:42 +0000 (16:36 +0000)]
ast_write() ignores ast_audiohook_write() results
In ast_write(), if a channel has a list of audiohooks, those
lists are written to and the resulting frame is what ast_write()
should continue with. The problem was the returned audiohook frame
was not being handled at all, and the original frame passed
into it did not contain the mixed audio, so essentially audio
was being lost. One result of this was chan_spy's whisper
mode no longer worked. To complicate the issue, frames
passed into ast_write may either be a single frame, or a list
of frames. So, as the list of frames is processed in the
audiohook_write, the returned frames had to be added to a new
list.
Tilghman Lesher [Tue, 25 Aug 2009 06:34:44 +0000 (06:34 +0000)]
Improve error message by informing user exactly which function is missing a parethesis.
(closes issue #15242)
Reported by: Nick_Lewis
Patches:
pbx.c-funcparenthesis.patch2 uploaded by dbrooks (license 790)
pbx.c-funcparenthesis-1.4.diff uploaded by loloski (license 68)
Tilghman Lesher [Fri, 21 Aug 2009 16:52:53 +0000 (16:52 +0000)]
Permit DEBUG_FD_LEAKS to be used with C++ source files.
(closes issue #15698)
Reported by: slavon
Patches:
20090817__issue15698.diff.txt uploaded by tilghman (license 14)
Tested by: slavon, tilghman
Sean Bright [Tue, 18 Aug 2009 16:36:00 +0000 (16:36 +0000)]
Delay the creation of temporary files until we have a valid manager command to handle.
Without this patch, asterisk creates a temporary file before determining if the
specified command is valid. If invalid, we weren't properly cleaning up the file.
Jeff Peeler [Mon, 17 Aug 2009 16:34:56 +0000 (16:34 +0000)]
Fix segfault when reloading chan_misdn.
If more ports were specified than configured in misdn.conf a reload would crash
asterisk. The problem was the unconfigured port was using data from the
previously configured port. When the data for an unconfigured port was freed a
crash would result from the double free.
Mark Michelson [Wed, 12 Aug 2009 18:46:09 +0000 (18:46 +0000)]
Backport fix so that outbound CANCEL requests have same branch as challenged INVITEs.
There already was code present to be sure that a CANCEL will contain the same branch-id
as the INVITE it is cancelling. However, for INVITES which are challenged downstream,
this mechanism did not work properly. Now this is taken care of.
This is a backport of a fix already present in all 1.6.X branches and in trunk. It also
fixes ABE-1907.
Tilghman Lesher [Fri, 7 Aug 2009 18:16:28 +0000 (18:16 +0000)]
QUEUE_MEMBER_LIST _really_ wants the interface name, not the membername.
This is a partial revert of revision 82590, which was an attempted cleanup,
but in reality, it broke QUEUE_MEMBER_LIST, which has always been intended
as a method by which component interfaces could be queried from the queue.
Membername isn't useful here, because that field cannot be used to obtain
further information about the member. See the documentation on
QUEUE_MEMBER_LIST, RemoveQueueMember, QUEUE_MEMBER_PENALTY, and the various
AMI commands which take a member argument for further justification.
(closes issue #15664)
Reported by: rain
Patches:
app_queue-queue_member_list.diff uploaded by rain (license 327)
Tilghman Lesher [Thu, 6 Aug 2009 21:45:01 +0000 (21:45 +0000)]
Because channel information can be accessed outside of the channel thread, we must lock the channel prior to modifying it.
(closes issue #15397)
Reported by: caspy
Patches:
20090714__issue15397.diff.txt uploaded by tilghman (license 14)
Tested by: caspy
Richard Mudgett [Wed, 5 Aug 2009 19:18:56 +0000 (19:18 +0000)]
Dialplan starts execution before the channel setup is complete.
* Issue 15655: For the case where dialing is complete for an incoming
call, dahdi_new() was asked to start the PBX and then the code set more
channel variables. If the dialplan hungup before these channel variables
got set, asterisk would likely crash.
* Fixed potential for overlap incoming call to erroneously set channel
variables as global dialplan variables if the ast_channel structure failed
to get allocated.
* Added missing set of CALLINGSUBADDR in the dialing is complete case.
Leif Madsen [Wed, 5 Aug 2009 18:46:21 +0000 (18:46 +0000)]
Update imapstorage.txt documentation.
Updated the imapstorage.txt documentation to reflect that issues with
c-client versions older than 2007 seem to cause crashing issues that
are not seen with more recent versions. Documentation has been updated
to reflect this.
Eliminate spurious compiler warnings from system headers on *BSD platforms.
Ensure that system headers located in /usr/local/include are actually treated
as system headers by the compiler, and not as local headers which are subject
to warnings from the -Wundef compiler option and others.
David Brooks [Mon, 3 Aug 2009 16:15:20 +0000 (16:15 +0000)]
Fixes dialplan wildcard extension taking precedence over call pickup code.
Prior to this patch, a wildcard extension in the dialplan (for example, _*.) would take
precedence over picking up a call in the channel's pickup group. This patch simply moves
the block of code handling pickup group matching to above the extension matching code.