[SR-Users] [PATCH] pv: make set_var_value handle overlapping memory

[Daniel-Constantin Mierla]

Hello,

I pushed a patch for fixing this issue on master branch -- can you test 
and report if works fine for you now?

Cheers,
Daniel

On 5/3/13 1:07 AM, Daniel-Constantin Mierla wrote:
> Hello,
>
> this patch is fixing only for $var(...), but there are other variables 
> that can be in the same situation. All PV-set functions have to be 
> updated as well as could be the case for config interpreter and some 
> core functions.
>
> Anyhow, going for an possible unoptimized operation all the time is 
> not recommended. It should be fixed where it creates the problem, not 
> restrict everything to something that avoids the problem.
>
> Cheers,
> Daniel
>
> On 5/2/13 7:57 PM, Martin Mikkelsen wrote:
>> set_var_value() may do memory corruption when a variable is set to the
>> substring of the same variable, this is atleast the case with the
>> s.substr, s.select, s.strip and s.striptail transformation functions
>> which just updates the pointer to its input buffer.
>>
>> As an example, this would trigger a warning in valgrind and memory
>> corruption because the memory areas overlap:
>>
>>    $var(x) = $(var(x){s.substr,1,0});
>>
>> Fix this by using memove() to do the copying instead of strncpy(), also
>> fix the add_var() function, there is no point in using strncpy when we
>> allready know the length of the variable so use memcpy() instead.
>> ---
>>   modules/pv/pv_svar.c |    7 +++++--
>>   1 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/modules/pv/pv_svar.c b/modules/pv/pv_svar.c
>> index bb76a1d..7ea01d0 100644
>> --- a/modules/pv/pv_svar.c
>> +++ b/modules/pv/pv_svar.c
>> @@ -66,7 +66,7 @@ script_var_t* add_var(str *name)
>>           return 0;
>>       }
>>       it->name.len = name->len;
>> -    strncpy(it->name.s, name->s, name->len);
>> +    memcpy(it->name.s, name->s, name->len);
>>       it->name.s[it->name.len] = '\0';
>>         it->next = script_vars;
>> @@ -119,7 +119,10 @@ script_var_t* set_var_value(script_var_t* var, 
>> int_str *value, int flags)
>>               }
>>               var->v.flags |= VAR_VAL_STR;
>>           }
>> -        strncpy(var->v.value.s.s, value->s.s, value->s.len);
>> +        if (var->v.value.s.s != value->s.s)
>> +        {
>> +            memmove(var->v.value.s.s, value->s.s, value->s.len);
>> +        }
>>           var->v.value.s.len = value->s.len;
>>           var->v.value.s.s[value->s.len] = '\0';
>>       } else {
>

-- 
Daniel-Constantin Mierla - http://www.asipto.com
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
Kamailio Advanced Training, San Francisco, USA - June 24-27, 2013
   * http://asipto.com/u/katu *