[sr-dev] git:master:dc2acb89: core: replace glibc time function calls with the thread-safe versions

[Daniel-Constantin Mierla]

Hello,

were these changes to use ctime_r() (and in other commits gmtime_r(),
localtime_r(), asctime_r()) required to fix existing issues or more like
a preference to use them? Because Kamailio is multiprocess and thread
safety should not be a concern, otherwise there might be a lot of other
place to take care of thread safety...

Anyhow, there are out of bound writes introduced as the buffers must be
at least 26 chars long, not 25 -- those need to be fixed.

Cheers,
Daniel

On 20.09.19 00:04, Henning Westerholt wrote:
> Module: kamailio
> Branch: master
> Commit: dc2acb895538131e99c770da6f7448cb5a46fc32
> URL: https://github.com/kamailio/kamailio/commit/dc2acb895538131e99c770da6f7448cb5a46fc32
>
> Author: Henning Westerholt <hw at skalatan.de>
> Committer: Henning Westerholt <hw at skalatan.de>
> Date: 2019-09-19T23:49:32+02:00
>
> core: replace glibc time function calls with the thread-safe versions
>
> - replace glibc time function calls with the thread-safe versions, to prevent
>   race conditions from multi-process / multi-threaded access
> - used in 'kamcmd core.uptime' rpc cmd, no functional change, locally tested
>
> ---
>
> Modified: src/core/core_cmd.c
>
> ---
>
> Diff:  https://github.com/kamailio/kamailio/commit/dc2acb895538131e99c770da6f7448cb5a46fc32.diff
> Patch: https://github.com/kamailio/kamailio/commit/dc2acb895538131e99c770da6f7448cb5a46fc32.patch
>
> ---
>
> diff --git a/src/core/core_cmd.c b/src/core/core_cmd.c
> index 5b1c4624ed..717e240fde 100644
> --- a/src/core/core_cmd.c
> +++ b/src/core/core_cmd.c
> @@ -224,8 +224,7 @@ static const char* dst_blst_stats_get_doc[] = {
>  #endif
>  
>  
> -
> -#define MAX_CTIME_LEN 128
> +#define MAX_CTIME_LEN 25
>  
>  /* up time */
>  static char up_since_ctime[MAX_CTIME_LEN];
> @@ -381,13 +380,14 @@ static void core_uptime(rpc_t* rpc, void* c)
>  {
>  	void* s;
>  	time_t now;
> +	char buf[MAX_CTIME_LEN];
>  	str snow;
> +	snow.s = buf;
>  
>  	time(&now);
>  
>  	if (rpc->add(c, "{", &s) < 0) return;
> -	snow.s = ctime(&now);
> -	if(snow.s) {
> +	if(ctime_r(&now, snow.s)) {
>  		snow.len = strlen(snow.s);
>  		if(snow.len>2 && snow.s[snow.len-1]=='\n') snow.len--;
>  		rpc->struct_add(s, "S", "now", &snow);
> @@ -1187,21 +1187,14 @@ int register_core_rpcs(void)
>  
>  int rpc_init_time(void)
>  {
> -	char *t;
> -	t=ctime(&up_since);
> -	if (strlen(t)+1>=MAX_CTIME_LEN) {
> -		ERR("Too long data %d\n", (int)strlen(t));
> +	char t[MAX_CTIME_LEN];
> +	int len;
> +	if (! ctime_r(&up_since, t)) {
> +		ERR("Invalid time value\n");
>  		return -1;
>  	}
>  	strcpy(up_since_ctime, t);
> -	t = up_since_ctime + strlen(up_since_ctime);
> -	while(t>up_since_ctime) {
> -		if(*t=='\0' || *t=='\r' || *t=='\n') {
> -			*t = '\0';
> -		} else {
> -			break;
> -		}
> -		t--;
> -	}
> +	len = strlen(up_since_ctime);
> +	if(len>2 && up_since_ctime[len-1]=='\n') up_since_ctime[len-1]='\0';
>  	return 0;
>  }
>
>
> _______________________________________________
> Kamailio (SER) - Development Mailing List
> sr-dev at lists.kamailio.org
> https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev

-- 
Daniel-Constantin Mierla -- www.asipto.com
www.twitter.com/miconda -- www.linkedin.com/in/miconda
Kamailio Advanced Training, Oct 21-23, 2019, Berlin, Germany -- https://asipto.com/u/kat