[Freeswitch-dev] Switch_core_get_variable

[Kevin Snow]

I can enter it into jira.

I¹m not sure if I agree with your assessment. There is already a mutex
lock/unlock around switch_core_get_variable¹s internals today, no additional
risk of deadlock. As for switch_channel_get_variable, which also has a mutex
of it¹s own, is already using switch_core_session_strdup before returning.
I¹m proposing the equivalent to switch_channel_get_variable_dup be added for
switch_core_get_variable. To me it seems we must have this if you want to
reliably read a variable that might change at runtime.

Kevin




On 2/2/11 12:01 PM, "Steven Ayre" <steveayre at gmail.com> wrote:

>> I found a hole in the way we handle core variables in FS.
>> Switch_core_get_variable does the lookup and returns the found pointer. In
>> Switch_core_set_variable the first step it does is look up the variable and
>> free it (if it exists). This would free it out from under another that has
>> just done a get on it. This is how I stumbled on this. 
> 
> The correct place to report bugs is the bug
> tracker: http://jira.freeswitch.org/
> 
> It's a known problem. Basically the answer is you shouldn't really be changing
> core variables after FS has loaded, at least not while handling traffic. If
> you're doing so as part of your application, you're doing it wrong. mod_db is
> probably better suited to what you want to do.
> 
>> Is the right fix is to add a switch_core_get_variable_dup that dups the
>> string while in the mutex protection? I realize in the core case this will
>> require the caller to then free the returned memory, but this is better than
>> getting a bad pointer. This is analogous to the switch_channel_get_variable
>> and it¹s _dup implementation, although it dups it to session memory.
> 
> Just my 2 cents (I'm not a core developer)... That could be done. But it's
> also slower because it means dynamically assigning memory and then copying the
> string. And it can't be cached. And the mutex will make all calls block, not
> just the single call as with the channel variable - which might possibly
> introduce a risk of deadlock.
> 
> -Steve
> 
> 
> 
> On 2 February 2011 19:39, Kevin Snow <kevin.snow at ooma.com> wrote:
>> 
>> I found a hole in the way we handle core variables in FS.
>> Switch_core_get_variable does the lookup and returns the found pointer. In
>> Switch_core_set_variable the first step it does is look up the variable and
>> free it (if it exists). This would free it out from under another that has
>> just done a get on it. This is how I stumbled on this.
>> 
>> Is the right fix is to add a switch_core_get_variable_dup that dups the
>> string while in the mutex protection? I realize in the core case this will
>> require the caller to then free the returned memory, but this is better than
>> getting a bad pointer. This is analogous to the switch_channel_get_variable
>> and it¹s _dup implementation, although it dups it to session memory.
>> 
>> Switch_channel_get_variable¹s ability to peak through to the core variables
>> is susceptible to this. If a core variable is changed after
>> switch_channel_get_variable looks it up but before it dups to the session
>> pool, it¹ll have a bad pointer.
>> 
>> Kevin Snow
>> Ooma, Inc 
>> 
>> _______________________________________________
>> FreeSWITCH-dev mailing list
>> FreeSWITCH-dev at lists.freeswitch.org
>> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
>> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
>> http://www.freeswitch.org
>> 
> 
> 
> 
> _______________________________________________
> FreeSWITCH-dev mailing list
> FreeSWITCH-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.freeswitch.org/pipermail/freeswitch-dev/attachments/20110202/7052eaed/attachment.html