Mark Michelson [Tue, 18 Mar 2008 23:32:58 +0000 (23:32 +0000)]
Fix a typo which caused a double free in chan_zap. This was discovered
by Juggie while attempting to load chan_zap. Apparently this would happen
if an error were encountered while trying to process zapata.conf.
Kevin P. Fleming [Tue, 18 Mar 2008 22:32:26 +0000 (22:32 +0000)]
start the process of changing HTTP request dispatching to do it based on *both* URI and method, so that POST support can move into a module; move http.c's private function prototypes into _private.h
This patch makes it so that all queue member status changes are handled through device state
code. This removes several problems people were seeing where their queue members would get into
an "unknown" state. Huge props go to atis on this one since he was the one who found the code
section that was causing the problem and proposed the solution. I just wrote what he suggested :)
Mark Michelson [Tue, 18 Mar 2008 18:58:42 +0000 (18:58 +0000)]
Add option 'randomperiodicannounce' to queues.conf. Setting this will
allow the list of periodic announcments specified to be played in a random
order instead of being played sequentially.
Terry Wilson [Tue, 18 Mar 2008 15:43:34 +0000 (15:43 +0000)]
Go through and fix a bunch of places where character strings were being interpreted as format strings. Most of these changes are solely to make compiling with -Wsecurity and -Wformat=2 happy, and were not
actual problems, per se. I also added format attributes to any printf wrapper functions I found that didn't have them. -Wsecurity and -Wmissing-format-attribute added to --enable-dev-mode.
Many thanks to atis for spotting this problem and reporting it.
The fix was to straighten out how items are placed on and removed
from the file stack. Regressions as well as the provided test case
helped to straighten out all code paths. valgrind was used to make
sure all memory allocated was freed.
Sorry for not solving this earlier. I got distracted.
Added the ntest23 regression test, which is mainly a copy of ntest22,
but with a few juicy errors thrown in, to replicate the kind of
error that atis spotted.
Terry Wilson [Mon, 17 Mar 2008 22:10:06 +0000 (22:10 +0000)]
Replace minimime with superior GMime library so that the entire contents of an http post are not read into memory.
This does introduce a dependency on the GMime library for handling HTTP POSTs, but it is available in most distros.
If the library is present, then the compile flag for ENABLE_UPLOADS is enabled by default in menuselect.
Fix a logic flaw in the code that stores lock info which is displayed
via the "core show locks" command. The idea behind this section of code was
to remove the previous lock from the list if it was a trylock that had failed.
Unfortunately, instead of checking the status of the previous lock, we were referencing
the index immediately following the previous lock in the lock_info->locks array.
The result of this problem, under the right circumstances, was that the lock which
we currently in the process of attempting to acquire could "overwrite" the previous lock
which was acquired. While this does not in any way affect typical operation, it *could*
lead to misleading "core show locks" output.
Due to a bug that occurred when merge_contexts_and_delete scanned the "old" or existing contexts, and found a context
that doesn't exist in the new set, yet owned by a different registrar. The context is created in the new set, with the
old registrar, and and all the priorities and extens that have a different registrar are copied into it. But, not the
includes, ignorepats, and switches. I added code to do this immediately after the context is created.
This still leaves a logical hole in the code. If you define a context in two places, (eg. in extensions.conf and also
in extensions.ael), and they both have includes, but different in composition, no new context will be generated, and
therefore the 'old' includes, switches, and ignorepats will not be copied. I'd have added code to simply add any non-duplicates
into the 'new' context that had a different registrar, but there is one big complication: includes, and switches are definitely
order dependent. (ignorepats I'm not sure about). And we'll have to develop some sort of policy about how we
merge order dependent lists, especially if the intersection of the two sets is empty. (in other words, they do not have any
elements in common). Do the new go first, or the old? I've elected to punt this issue until a user complains. Hopefully,
this is pretty rare thing.
200 OKs in response to a reinvite need to be sent reliably. If the remote side does not receive one the dialog will be torn down.
(closes issue #12208)
Reported by: atrash
Joshua Colp [Mon, 17 Mar 2008 14:37:40 +0000 (14:37 +0000)]
Make sure that the temporary sip_request structure is empty so that copy_request doesn't think it already has an ast_str.
(closes issue #12231)
Reported by: IgorG
Fix a race condition in the SIP packet scheduler which could cause a crash.
chan_sip uses the scheduler API in order to schedule retransmission of reliable
packets (such as INVITES). If a retransmission of a packet is occurring, then the
packet is removed from the scheduler and retrans_pkt is called. Meanwhile, if
a response is received from the packet as previously transmitted, then when we
ACK the response, we will remove the packet from the scheduler and free the packet.
The problem is that both the ACK function and retrans_pkt attempt to acquire the
same lock at the beginning of the function call. This means that if the ACK function
acquires the lock first, then it will free the packet which retrans_pkt is about to
read from and write to. The result is a crash.
The solution:
1. If the ACK function fails to remove the packet from the scheduler and the retransmit
id of the packet is not -1 (meaning that we have not reached the maximum number of
retransmissions) then release the lock and yield so that retrans_pkt may acquire the
lock and operate.
2. Make absolutely certain that the ACK function does not recursively lock the lock in
question. If it does, then releasing the lock will do no good, since retrans_pkt will
still be unable to acquire the lock.
Russell Bryant [Thu, 13 Mar 2008 21:40:43 +0000 (21:40 +0000)]
Merged revisions 108583 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r108583 | russell | 2008-03-13 16:38:16 -0500 (Thu, 13 Mar 2008) | 11 lines
Fix another issue that was causing crashes in chanspy. This introduces a new
datastore callback, called chan_fixup(). The concept is exactly like the
fixup callback that is used in the channel technology interface. This callback
gets called when the owning channel changes due to a masquerade. Before this
was introduced, if a masquerade happened on a channel being spyed on, the
channel pointer in the datastore became invalid.
(closes issue #12187)
(reported by, and lots of testing from atis)
(props to file for the help with ideas)
Russell Bryant [Thu, 13 Mar 2008 21:06:52 +0000 (21:06 +0000)]
Merged revisions 108530 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r108530 | russell | 2008-03-13 16:06:33 -0500 (Thu, 13 Mar 2008) | 10 lines
Make a tweak that gets the LEDs on polycom phones to blink when an extension that
has been subscribed to goes on hold. Otherwise, they just stay on like it does
when an extension is in use.
(closes issue #11263)
Reported by: russell
Patches:
notify_hold.rev1.txt uploaded by russell (license 2)
Tested by: russell
Mark Michelson [Thu, 13 Mar 2008 20:59:00 +0000 (20:59 +0000)]
Fixing a potential buffer overflow in the manager command ModuleCheck.
Though this overflow is exploitable remotely, we are NOT issuing a security
advisory for this since in order to exploit the overflow, the attacker would
have to establish an authenticated manager session AND have the system privilege.
By gaining this privilege, the attacker already has more powerful weapons at his
disposal than overflowing a buffer with a malformed manager header, so the vulnerability
in this case really lies with the authentication method that allowed the attacker to
gain the system privilege in the first place.
Russell Bryant [Thu, 13 Mar 2008 20:26:59 +0000 (20:26 +0000)]
Merged revisions 108469 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r108469 | russell | 2008-03-13 15:26:28 -0500 (Thu, 13 Mar 2008) | 4 lines
Fix a couple uses of sprintf. The second one could actually cause an overflow
of a stack buffer. It's not a security issue though, it only depends on your
configuration.
Russell Bryant [Thu, 13 Mar 2008 19:54:44 +0000 (19:54 +0000)]
Merge changes from team/jamesgolovich/chan_sip-ast_str
This set of changes removes the hard coded maximum packet size of 4kB from chan_sip.
It now starts by allocating 1kB, and growing the buffer as needed to accommodate large
packets.
(closes issue #8556, reported by mikma, patch by jamesgolovich)
Jeff Peeler [Thu, 13 Mar 2008 18:59:04 +0000 (18:59 +0000)]
(closes issue #11827)
Reported by: ctooley
Patches:
eivr_tcp_generic.patch uploaded by jpeeler (license 325)
This change adds the ability to communicate over a TCP socket instead of forking a child process.
Change AST_SCHED_DEL use to ast_sched_del for autocongestion in chan_sip.
The scheduler callback will always return 0. This means that this id
is never rescheduled, so it makes no sense to loop trying to delete
the id from the scheduler queue. If we fail to remove the item from the
queue once, it will fail every single time.
(Yes I realize that in this case, the macro would exit early because the
id is set to -1 in the callback, but it still makes no sense to use
that macro in favor of calling ast_sched_del once and being done with it)
This is the first of potentially several such fixes.
Added a large comment before the AST_SCHED_DEL macro to explain its purpose as well as when
it is appropriate and when it is not appropriate to use it.
I also removed the part of the debug message that mentions that this is probably a bug because
there are some perfectly legitimate places where ast_sched_del may fail to delete an entry (e.g.
when the scheduler callback manually reschedules with a new id instead of returning non-zero to
tell the scheduler to reschedule with the same idea). I also raised the debug level of the debug
message in AST_SCHED_DEL since it seems like it could come up quite frequently since the macro
is probably being used in several places where it shouldn't be. Also removed the redundant line,
file, and function information since that is provided by ast_log.
if we receive an INVITE with a Content-Length that is not a valid number, or is zero, then don't process the rest of the message body looking for an SDP
Russell Bryant [Wed, 12 Mar 2008 19:59:05 +0000 (19:59 +0000)]
Merged revisions 108135 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r108135 | russell | 2008-03-12 14:57:42 -0500 (Wed, 12 Mar 2008) | 40 lines
(closes issue #12187, reported by atis, fixed by me after some brainstorming
on the issue with mmichelson)
- Update copyright info on app_chanspy.
- Fix a race condition that caused app_chanspy to crash. The issue was that
the chanspy datastore magic that was used to ensure that spyee channels did
not disappear out from under the code did not completely solve the problem.
It was actually possible for chanspy to acquire a channel reference out of
its datastore to a channel that was in the middle of being destroyed. That
was because datastore destruction in ast_channel_free() was done near the
end. So, this left the code in app_chanspy accessing a channel that was
partially, or completely invalid because it was in the process of being free'd
by another thread. The following sort of shows the code path where the race
occurred:
=============================================================================
Thread 1 (PBX thread for spyee chan) || Thread 2 (chanspy)
--------------------------------------||-------------------------------------
ast_channel_free() ||
- remove channel from channel list ||
- lock/unlock the channel to ensure ||
that no references retrieved from ||
the channel list exist. ||
--------------------------------------||-------------------------------------
|| channel_spy()
- destroy some channel data || - Lock chanspy datastore
|| - Retrieve reference to channel
|| - lock channel
|| - Unlock chanspy datastore
--------------------------------------||-------------------------------------
- destroy channel datastores ||
- call chanspy datastore d'tor ||
which NULL's out the ds' || - Operate on the channel ...
reference to the channel ||
||
- free the channel ||
||
|| - unlock the channel
--------------------------------------||-------------------------------------
=============================================================================
Add a trigger mode that triggers on both read and write. The actual function that returns the combined audio frame though will wait until both sides have fed in audio, or until one side stops (such as the case when you call Wait).
(closes issue #11945)
Reported by: xheliox
Russell Bryant [Wed, 12 Mar 2008 17:06:37 +0000 (17:06 +0000)]
- Add Tilghman to the copyright info ... he wrote the hard part :)
- Remove some magic in unload_module that isn't needed. Module use counts
already ensure that the function isn't going to be in use at this point.
Tilghman Lesher [Tue, 11 Mar 2008 22:55:16 +0000 (22:55 +0000)]
An offhand comment from Russell made me realize that the configuration file
caching would not work properly for users.conf and any other file read from
more than one place. I needed to add the filename which requested the config
file to get it to work properly.
Russell Bryant [Tue, 11 Mar 2008 22:21:19 +0000 (22:21 +0000)]
Add a trivial new dialplan function, AST_CONFIG(), which allows you to access
a variable from an Asterisk configuration file in the dialplan, or anywhere
else where dialplan functions can be used.
Joshua Colp [Tue, 11 Mar 2008 20:36:14 +0000 (20:36 +0000)]
Dial a device even if it's state is unknown.
(closes issue #12184)
Reported by: bluecrow76
Patches:
asterisk-svn-app_page.c.devicestate_unknown.diff uploaded by bluecrow76 (license 270)
Make sure the visible indication is on the right channel so when the masquerade happens the proper indication is enacted.
(closes issue #11707)
Reported by: iam
Add an additional check for setting conference parameter when using the marked user options. It was possible for it to return to a no listen/no talk state if a masquerade happened.
(closes issue #12136)
Reported by: aragon
fix up various compiler warnings found with gcc-4.3:
- the output of flex includes a static function called 'input' that is not used, so for the moment we'll stop having the compiler tell us about unused variables in the flex source files (a better fix would be to improve our flex post-processing to remove the unused function)
- main/stdtime/localtime.c makes assumptions about signed integer overflow, and gcc-4.3's improved optimizer tries to take advantage of handling potential overflow conditions at compile time; for now, suppress these optimizations until we can fiure out if the code needs improvement
- main/udptl.c has some references to uninitialized variables; in one case there was no bug, but in the other it was certainly possibly for unexpected behavior to occur
Russell Bryant [Mon, 10 Mar 2008 20:17:37 +0000 (20:17 +0000)]
Merged revisions 107161 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4
........
r107161 | russell | 2008-03-10 15:17:11 -0500 (Mon, 10 Mar 2008) | 8 lines
Fix another bug specifically related to asynchronous call origination. Once the
PBX is started on the channel using ast_pbx_start(), then the ownership of the
channel has been passed on to another thread. We can no longer access it in this
code. If the channel gets hung up very quickly, it is possible that we could
access a channel that has been free'd.
projects / thirdparty / asterisk.git / log
