Skip to content

Commit 86a2cee

Browse files
committed
Fix nfprofile race condition and therefore replace all remaining localtime() by localtime_r()
1 parent 68a18af commit 86a2cee

21 files changed

Lines changed: 166 additions & 164 deletions

File tree

src/collector/bookkeeper.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -549,7 +549,7 @@ void UpdateBooksParam(bookkeeper_t *bookkeeper, time_t lifetime, uint64_t maxsiz
549549

550550
void PrintBooks(bookkeeper_t *bookkeeper) {
551551
bookkeeper_list_t *bookkeeper_list_entry;
552-
struct tm *ts;
552+
struct tm ts;
553553
time_t t;
554554
char string[32];
555555

@@ -570,14 +570,14 @@ void PrintBooks(bookkeeper_t *bookkeeper) {
570570
printf("Record sequence : %llu\n", (unsigned long long)bookkeeper->sequence);
571571

572572
t = bookkeeper->first;
573-
ts = localtime(&t);
574-
strftime(string, 31, "%Y-%m-%d %H:%M:%S", ts);
573+
localtime_r(&t, &ts);
574+
strftime(string, 31, "%Y-%m-%d %H:%M:%S", &ts);
575575
string[31] = '\0';
576576
printf("First : %s\n", bookkeeper->first ? string : "<not set>");
577577

578578
t = bookkeeper->last;
579-
ts = localtime(&t);
580-
strftime(string, 31, "%Y-%m-%d %H:%M:%S", ts);
579+
localtime_r(&t, &ts);
580+
strftime(string, 31, "%Y-%m-%d %H:%M:%S", &ts);
581581
string[31] = '\0';
582582
printf("Last : %s\n", bookkeeper->last ? string : "<not set>");
583583
printf("Number of files : %llu\n", (unsigned long long)bookkeeper->numfiles);

src/collector/collector.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,9 +417,10 @@ int RotateCycle(const collector_ctx_t *ctx, post_args_t *post_args, time_t t_sta
417417
static int RunCycle(time_t t_start, const char *time_extension, const collector_ctx_t *ctx, int *pfd, int done, uint32_t creator,
418418
uint32_t compression, uint32_t encryption) {
419419
// periodic file rotation
420-
struct tm *now = localtime(&t_start);
420+
struct tm now;
421+
localtime_r(&t_start, &now);
421422
char fmt[32];
422-
strftime(fmt, sizeof(fmt), time_extension, now);
423+
strftime(fmt, sizeof(fmt), time_extension, &now);
423424

424425
dbg_printf("Enter RunCycle\n");
425426

@@ -431,7 +432,7 @@ static int RunCycle(time_t t_start, const char *time_extension, const collector_
431432
char nfcapd_filename[MAXPATHLEN];
432433
nfcapd_filename[0] = '\0';
433434

434-
int pos = SetupPath(now, fs->datadir, fs->subdir, nfcapd_filename);
435+
int pos = SetupPath(&now, fs->datadir, fs->subdir, nfcapd_filename);
435436
char *p = nfcapd_filename + (ptrdiff_t)pos;
436437
snprintf(p, MAXPATHLEN - pos - 1, "nfcapd.%s", fmt);
437438
nfcapd_filename[MAXPATHLEN - 1] = '\0';

src/collector/expire.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -366,16 +366,16 @@ void ExpireDir(char *dir, dirstat_t *dirstat, uint64_t maxsize, uint64_t maxlife
366366
// this means all files will get expired - are you sure ?
367367
char *s, s1[32], s2[32];
368368
time_t t;
369-
struct tm *when;
369+
struct tm when;
370370

371371
t = t_expire;
372-
when = localtime(&t);
373-
strftime(s1, 31, "%Y-%m-%d %H:%M:%S", when);
372+
localtime_r(&t, &when);
373+
strftime(s1, 31, "%Y-%m-%d %H:%M:%S", &when);
374374
s1[31] = '\0';
375375

376376
t = dirstat->last;
377-
when = localtime(&t);
378-
strftime(s2, 31, "%Y-%m-%d %H:%M:%S", when);
377+
localtime_r(&t, &when);
378+
strftime(s2, 31, "%Y-%m-%d %H:%M:%S", &when);
379379
s2[31] = '\0';
380380

381381
printf("Your max lifetime of %s will expire all file before %s\n", ScaleTime(maxlife), s1);

src/collector/nfstatfile.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -505,19 +505,19 @@ int ReleaseStatInfo(dirstat_t *dirstat) {
505505
} // End of ReleaseStatInfo
506506

507507
void PrintDirStat(dirstat_t *dirstat) {
508-
struct tm *ts;
508+
struct tm ts;
509509
time_t t;
510510
char string[32];
511511

512512
t = dirstat->first;
513-
ts = localtime(&t);
514-
strftime(string, 31, "%Y-%m-%d %H:%M:%S", ts);
513+
localtime_r(&t, &ts);
514+
strftime(string, 31, "%Y-%m-%d %H:%M:%S", &ts);
515515
string[31] = '\0';
516516
printf("First: %s\n", string);
517517

518518
t = dirstat->last;
519-
ts = localtime(&t);
520-
strftime(string, 31, "%Y-%m-%d %H:%M:%S", ts);
519+
localtime_r(&t, &ts);
520+
strftime(string, 31, "%Y-%m-%d %H:%M:%S", &ts);
521521
string[31] = '\0';
522522
printf("Last: %s\n", string);
523523

src/libnfdump/tor/tor.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,10 @@ int Init_TorLookup(void) {
7070
} // End of Init_TorLookup
7171

7272
static char *tmString(time_t time, char *buff, size_t len) {
73-
struct tm *tmTime = localtime(&time);
74-
snprintf(buff, len, "%4d-%02d-%02d %02d:%02d:%02d", tmTime->tm_year + 1900, tmTime->tm_mon + 1, tmTime->tm_mday, tmTime->tm_hour, tmTime->tm_min,
75-
tmTime->tm_sec);
73+
struct tm tmTime;
74+
localtime_r(&time, &tmTime);
75+
snprintf(buff, len, "%4d-%02d-%02d %02d:%02d:%02d", tmTime.tm_year + 1900, tmTime.tm_mon + 1, tmTime.tm_mday, tmTime.tm_hour, tmTime.tm_min,
76+
tmTime.tm_sec);
7677
return buff;
7778
}
7879

src/libnffile/flist.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
*
3030
*/
3131

32+
#include <assert.h>
3233
#include <ctype.h>
3334
#include <dirent.h>
3435
#include <errno.h>
@@ -1043,15 +1044,16 @@ static char *VerifyFileRange(char *path, char *last_file) {
10431044

10441045
static char *GuessSubDir(char *channeldir, char *filename) {
10451046
char s[MAXPATHLEN];
1046-
struct tm *t_tm;
1047+
struct tm t_tm = {0};
10471048

10481049
dbg_printf("GuessSubDir() for file: %s in path: %s\n", filename, channeldir);
10491050

10501051
size_t len = strlen(filename);
1052+
assert(len == 19);
10511053
if ((len == 19 || len == 21) && (strncmp(filename, "nfcapd.", 7) == 0)) {
10521054
char *p = &filename[7];
10531055
time_t t = ISO2UNIX(p);
1054-
t_tm = localtime(&t);
1056+
localtime_r(&t, &t_tm);
10551057
} else
10561058
return NULL;
10571059

@@ -1062,7 +1064,7 @@ static char *GuessSubDir(char *channeldir, char *filename) {
10621064
char const *sub_fmt = subdir_def[i];
10631065
char subpath[255];
10641066
struct stat stat_buf;
1065-
strftime(subpath, 254, sub_fmt, t_tm);
1067+
strftime(subpath, 254, sub_fmt, &t_tm);
10661068
subpath[254] = '\0';
10671069

10681070
snprintf(s, MAXPATHLEN - 1, "%s/%s/%s", channeldir, subpath, filename);

src/libnffile/nffile.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,9 +1659,10 @@ int QueryFile(char *filename, int verbose) {
16591659
fileHeader.creator = CREATOR_UNKNOWN;
16601660
}
16611661

1662-
struct tm *tbuff = localtime(&fileHeader.created);
1662+
struct tm tbuff;
1663+
localtime_r(&fileHeader.created, &tbuff);
16631664
char t1[64];
1664-
strftime(t1, 63, "%Y-%m-%d %H:%M:%S", tbuff);
1665+
strftime(t1, 63, "%Y-%m-%d %H:%M:%S", &tbuff);
16651666
printf("Created : %s\n", t1);
16661667
printf("Created by : %s\n", nf_creator[fileHeader.creator]);
16671668
printf("nfdump : %x\n", fileHeader.nfdversion);
@@ -2101,9 +2102,10 @@ void PrintGNUplotSumStat(nffile_t *nffile) {
21012102
if (dateString) {
21022103
dateString += 7;
21032104
time_t when = ISO2UNIX(dateString);
2104-
struct tm *ts = localtime(&when);
2105+
struct tm ts;
2106+
localtime_r(&when, &ts);
21052107
char datestr[64];
2106-
strftime(datestr, 63, "%Y-%m-%d %H:%M:%S", ts);
2108+
strftime(datestr, 63, "%Y-%m-%d %H:%M:%S", &ts);
21072109
printf("%s,%llu,%llu,%llu\n", datestr, (long long unsigned)nffile->stat_record->numflows, (long long unsigned)nffile->stat_record->numpackets,
21082110
(long long unsigned)nffile->stat_record->numbytes);
21092111
} else {

src/libnffile/output_short.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -69,27 +69,27 @@ static void stringEXgenericFlow(FILE *stream, record_map_t *r) {
6969
EXgenericFlow_t *genericFlow = (EXgenericFlow_t *)((void *)elementHeader + sizeof(elementHeader_t));
7070

7171
char datestr1[64], datestr2[64], datestr3[64];
72-
struct tm *ts;
72+
struct tm ts;
7373
time_t when = genericFlow->msecFirst / 1000LL;
7474
if (when == 0) {
7575
strncpy(datestr1, "<unknown>", 63);
7676
} else {
77-
ts = localtime(&when);
78-
strftime(datestr1, 63, "%Y-%m-%d %H:%M:%S", ts);
77+
localtime_r(&when, &ts);
78+
strftime(datestr1, 63, "%Y-%m-%d %H:%M:%S", &ts);
7979
}
8080

8181
when = genericFlow->msecLast / 1000LL;
8282
if (when == 0) {
8383
strncpy(datestr2, "<unknown>", 63);
8484
} else {
85-
ts = localtime(&when);
86-
strftime(datestr2, 63, "%Y-%m-%d %H:%M:%S", ts);
85+
localtime_r(&when, &ts);
86+
strftime(datestr2, 63, "%Y-%m-%d %H:%M:%S", &ts);
8787
}
8888

8989
if (genericFlow->msecReceived) {
9090
when = genericFlow->msecReceived / 1000LL;
91-
ts = localtime(&when);
92-
strftime(datestr3, 63, "%Y-%m-%d %H:%M:%S", ts);
91+
localtime_r(&when, &ts);
92+
strftime(datestr3, 63, "%Y-%m-%d %H:%M:%S", &ts);
9393
} else {
9494
datestr3[0] = '0';
9595
datestr3[1] = '\0';

src/libnffile/util.c

Lines changed: 57 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -556,22 +556,21 @@ char *TimeString(uint64_t msecStart, uint64_t msecEnd) {
556556

557557
if (msecStart) {
558558
time_t secs = msecStart / 1000;
559-
struct tm *tbuff = localtime(&secs);
560-
if (!tbuff) {
561-
LogError("localtime() error in %s line %d: %s", __FILE__, __LINE__, strerror(errno));
562-
return "Error time convert";
559+
struct tm tbuff;
560+
if (!localtime_r(&secs, &tbuff)) {
561+
LogError("localtime_r() error in %s line %d: %s", __FILE__, __LINE__, strerror(errno));
562+
return "0000-00-00 00:00:00";
563563
}
564564
char t1[64];
565-
strftime(t1, 63, "%Y-%m-%d %H:%M:%S", tbuff);
565+
strftime(t1, 63, "%Y-%m-%d %H:%M:%S", &tbuff);
566566

567567
secs = msecEnd / 1000;
568-
tbuff = localtime(&secs);
569-
if (!tbuff) {
570-
LogError("localtime() error in %s line %d: %s", __FILE__, __LINE__, strerror(errno));
571-
return "Error time convert";
568+
if (!localtime_r(&secs, &tbuff)) {
569+
LogError("localtime_r() error in %s line %d: %s", __FILE__, __LINE__, strerror(errno));
570+
return "0000-00-00 00:00:00";
572571
}
573572
char t2[64];
574-
strftime(t2, 63, "%Y-%m-%d %H:%M:%S", tbuff);
573+
strftime(t2, 63, "%Y-%m-%d %H:%M:%S", &tbuff);
575574

576575
snprintf(datestr, 254, "%s.%03d - %s.%03d", t1, (int)(msecStart % 1000), t2, (int)(msecEnd % 1000));
577576
} else {
@@ -582,87 +581,72 @@ char *TimeString(uint64_t msecStart, uint64_t msecEnd) {
582581
}
583582

584583
char *UNIX2ISO(time_t t) {
585-
struct tm *when;
584+
struct tm when;
586585
static char timestring[32];
587586

588-
when = localtime(&t);
589-
when->tm_isdst = -1;
590-
snprintf(timestring, 31, "%4i%02i%02i%02i%02i%02i", when->tm_year + 1900, when->tm_mon + 1, when->tm_mday, when->tm_hour, when->tm_min,
591-
when->tm_sec);
587+
localtime_r(&t, &when);
588+
when.tm_isdst = -1;
589+
snprintf(timestring, 31, "%4i%02i%02i%02i%02i%02i", when.tm_year + 1900, when.tm_mon + 1, when.tm_mday, when.tm_hour, when.tm_min, when.tm_sec);
592590
timestring[31] = '\0';
593591

594592
return timestring;
595593

596594
} // End of UNIX2ISO
597595

598-
time_t ISO2UNIX(char *timestring) {
599-
char c, *p;
600-
struct tm when;
601-
time_t t;
602-
603-
// let localtime fill in all default fields such as summer time, TZ etc.
604-
t = time(NULL);
605-
localtime_r(&t, &when);
606-
when.tm_sec = 0;
607-
when.tm_wday = 0;
608-
when.tm_yday = 0;
609-
when.tm_isdst = -1;
596+
// convert yyyyMMddhhmm[ss] -> time_t
597+
time_t ISO2UNIX(const char *timestring) {
598+
if (!timestring) {
599+
LogError("NULL time string");
600+
return (time_t)-1;
601+
}
610602

611603
size_t len = strlen(timestring);
612604
if (len != 12 && len != 14) {
613-
LogError("Wrong time format '%s'\n", timestring);
614-
return 0;
605+
LogError("Wrong time format '%s'", timestring);
606+
return (time_t)-1;
615607
}
616-
// 2019 05 05 12 00 (10)
617-
// year
618-
p = timestring;
619-
c = p[4];
620-
p[4] = '\0';
621-
when.tm_year = atoi(p) - 1900;
622-
p[4] = c;
623-
624-
// month
625-
p += 4;
626-
c = p[2];
627-
p[2] = '\0';
628-
when.tm_mon = atoi(p) - 1;
629-
p[2] = c;
630-
631-
// day
632-
p += 2;
633-
c = p[2];
634-
p[2] = '\0';
635-
when.tm_mday = atoi(p);
636-
p[2] = c;
637-
638-
// hour
639-
p += 2;
640-
c = p[2];
641-
p[2] = '\0';
642-
when.tm_hour = atoi(p);
643-
p[2] = c;
644-
645-
// minute
646-
p += 2;
647-
c = p[2];
648-
p[2] = '\0';
649-
when.tm_min = atoi(p);
650-
p[2] = c;
608+
609+
// ensure all characters are digits
610+
for (size_t i = 0; i < len; i++) {
611+
if (!isdigit((unsigned char)timestring[i])) {
612+
LogError("Invalid character in time string '%s'", timestring);
613+
return (time_t)-1;
614+
}
615+
}
616+
617+
struct tm when = {0};
618+
619+
// parse manually without modifying input
620+
when.tm_year = (timestring[0] - '0') * 1000 + (timestring[1] - '0') * 100 + (timestring[2] - '0') * 10 + (timestring[3] - '0') - 1900;
621+
when.tm_mon = ((timestring[4] - '0') * 10 + (timestring[5] - '0')) - 1;
622+
when.tm_mday = (timestring[6] - '0') * 10 + (timestring[7] - '0');
623+
when.tm_hour = (timestring[8] - '0') * 10 + (timestring[9] - '0');
624+
when.tm_min = (timestring[10] - '0') * 10 + (timestring[11] - '0');
625+
when.tm_sec = 0;
651626

652627
if (len == 14) {
653-
p += 2;
654-
when.tm_sec = atoi(p);
628+
when.tm_sec = (timestring[12] - '0') * 10 + (timestring[13] - '0');
655629
}
656630

657-
t = mktime(&when);
658-
if (t == -1) {
659-
LogError("Failed to convert string '%s'\n", timestring);
660-
return 0;
661-
} else {
662-
// printf("%s %s", timestring, ctime(&t));
663-
return t;
631+
when.tm_isdst = -1; // let mktime determine DST
632+
633+
// range validation
634+
if (when.tm_mon < 0 || when.tm_mon > 11 || when.tm_mday < 1 || when.tm_mday > 31 || when.tm_hour < 0 || when.tm_hour > 23 || when.tm_min < 0 ||
635+
when.tm_min > 59 || when.tm_sec < 0 || when.tm_sec > 60) {
636+
LogError("Out-of-range values in '%s'", timestring);
637+
return (time_t)-1;
638+
}
639+
640+
// comment
641+
// pthread_mutex_lock(&mktime_mutex);
642+
time_t t = mktime(&when);
643+
// pthread_mutex_unlock(&mktime_mutex);
644+
if (t == (time_t)-1) {
645+
LogError("Failed to convert string '%s'", timestring);
646+
return (time_t)-1;
664647
}
665648

649+
return t;
666650
} // End of ISO2UNIX
667651

668652
long getTick(void) {

src/libnffile/util.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ char *TimeString(uint64_t msecStart, uint64_t msecEnd);
140140

141141
char *UNIX2ISO(time_t t);
142142

143-
time_t ISO2UNIX(char *timestring);
143+
time_t ISO2UNIX(const char *timestring);
144144

145145
uint64_t ParseTime8601(const char *s);
146146

0 commit comments

Comments
 (0)