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 <[email protected]> > Committer: Henning Westerholt <[email protected]> > 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 > [email protected] > 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 _______________________________________________ Kamailio (SER) - Development Mailing List [email protected] https://lists.kamailio.org/cgi-bin/mailman/listinfo/sr-dev
