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

Daniel-Constantin Mierla miconda at gmail.com
Fri May 3 01:07:05 CEST 2013


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 *




More information about the sr-users mailing list