From 5459ecb40ace4f91d6d765f4b1b06156805b10d9 Mon Sep 17 00:00:00 2001 From: Sebastien Godard Date: Sun, 14 Nov 2010 13:24:57 +0100 Subject: [PATCH] Fix segfaults on bogus localtime input. The return code from localtime() function (and also gmtime() one) wasn't checked. In some (rare) cases, it can return a NULL pointer resulting in a segmentation fault. The return code is now checked, but no specific action is performed anyway. Original patch from Ivana Varekova from RedHat (04/10/2010): diff -up sysstat-9.0.6.1/sar.c.pom sysstat-9.0.6.1/sar.c --- sysstat-9.0.6.1/sar.c.pom 2009-10-17 15:08:21.000000000 +0200 +++ sysstat-9.0.6.1/sar.c 2010-10-04 12:21:13.383442188 +0200 @@ -247,7 +247,7 @@ void reverse_check_act(unsigned int act_ * @curr Index in array for current sample statistics. *************************************************************************** */ -void sar_get_record_timestamp_struct(int curr) +int sar_get_record_timestamp_struct(int curr) { struct tm *ltm; @@ -257,10 +257,17 @@ void sar_get_record_timestamp_struct(int rectime.tm_hour = record_hdr[curr].hour; rectime.tm_min = record_hdr[curr].minute; rectime.tm_sec = record_hdr[curr].second; + return 0; } else { ltm = localtime((const time_t *) &record_hdr[curr].ust_time); + + /* localtime procedure could not finish successful */ + if (ltm == NULL) + return 1; + rectime = *ltm; + return 0; } } @@ -312,13 +319,17 @@ int check_line_hdr(void) * @cur_time Timestamp string. *************************************************************************** */ -void set_record_timestamp_string(int curr, char *cur_time, int len) +int set_record_timestamp_string(int curr, char *cur_time, int len) { + int ret; /* Fill timestamp structure */ - sar_get_record_timestamp_struct(curr); + ret = sar_get_record_timestamp_struct(curr); + if (ret != 0) + return ret; /* Set cur_time date value */ strftime(cur_time, len, "%X", &rectime); + return 0; } /* @@ -407,6 +418,7 @@ int write_stats(int curr, int read_from_ int use_tm_end, int reset, unsigned int act_id) { int i; + int ret; unsigned long long itv, g_itv; static int cross_day = 0; static __nr_t cpu_nr = -1; @@ -423,9 +435,14 @@ int write_stats(int curr, int read_from_ } /* Set previous timestamp */ - set_record_timestamp_string(!curr, timestamp[!curr], 16); + ret = set_record_timestamp_string(!curr, timestamp[!curr], 16); + if (ret != 0) + return ret; + /* Set current timestamp */ - set_record_timestamp_string(curr, timestamp[curr], 16); + ret = set_record_timestamp_string(curr, timestamp[curr], 16); + if (ret != 0) + return ret; /* Check if we are beginning a new day */ if (use_tm_start && record_hdr[!curr].ust_time && @@ -569,8 +586,11 @@ int sar_print_special(int curr, int use_ { char cur_time[26]; int dp = 1; + int ret; - set_record_timestamp_string(curr, cur_time, 26); + ret = set_record_timestamp_string(curr, cur_time, 26); + if (ret != 0) + return ret; /* The record must be in the interval specified by -s/-e options */ if ((use_tm_start && (datecmp(&rectime, &tm_start) < 0)) || @@ -865,7 +885,8 @@ void read_stats_from_file(char from_file */ read_file_stat_bunch(act, 0, ifd, file_hdr.sa_nr_act, file_actlst); - sar_get_record_timestamp_struct(0); + if (sar_get_record_timestamp_struct(0)) + continue; } } while ((rtype == R_RESTART) || (rtype == R_COMMENT) || --- CHANGES | 1 + common.c | 5 ++++- sa_common.c | 5 +++-- sadf.c | 9 ++++++--- sar.c | 50 ++++++++++++++++++++++++++++++++++++++------------ 5 files changed, 52 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index 6871da8..4f1d2e1 100644 --- a/CHANGES +++ b/CHANGES @@ -3,6 +3,7 @@ Changes: xxxx/xx/xx: Version 9.1.7 - Sebastien Godard (sysstat orange.fr) * sar now tells sadc to read only the necessary groups of activities. + * [Ivana Varekova]: Fix segfaults on bogus localtime input. * sar manual page updated. 2010/11/10: Version 9.1.6 - Sebastien Godard (sysstat orange.fr) diff --git a/common.c b/common.c index 9d64eaa..48c8368 100644 --- a/common.c +++ b/common.c @@ -285,7 +285,10 @@ int print_gal_header(struct tm *rectime, char *sysname, char *release, char *e; int rc = 0; - if (((e = getenv(ENV_TIME_FMT)) != NULL) && !strcmp(e, K_ISO)) { + if (rectime == NULL) { + strcpy(cur_date, "?/?/?"); + } + else if (((e = getenv(ENV_TIME_FMT)) != NULL) && !strcmp(e, K_ISO)) { strftime(cur_date, sizeof(cur_date), "%Y-%m-%d", rectime); rc = 1; } diff --git a/sa_common.c b/sa_common.c index d0ecc2e..ab2e6e8 100644 --- a/sa_common.c +++ b/sa_common.c @@ -382,8 +382,9 @@ void get_file_timestamp_struct(unsigned int flags, struct tm *rectime, mktime(rectime); } else { - loc_t = localtime((const time_t *) &file_hdr->sa_ust_time); - *rectime = *loc_t; + if ((loc_t = localtime((const time_t *) &file_hdr->sa_ust_time)) != NULL) { + *rectime = *loc_t; + } } } diff --git a/sadf.c b/sadf.c index 3739127..77379e7 100644 --- a/sadf.c +++ b/sadf.c @@ -125,8 +125,9 @@ void sadf_get_record_timestamp_struct(int curr) { struct tm *ltm; - ltm = localtime((const time_t *) &record_hdr[curr].ust_time); - loctime = *ltm; + if ((ltm = localtime((const time_t *) &record_hdr[curr].ust_time)) != NULL) { + loctime = *ltm; + } if (!PRINT_TRUE_TIME(flags) || ((format != S_O_DB_OPTION) && (format != S_O_XML_OPTION))) { @@ -134,7 +135,9 @@ void sadf_get_record_timestamp_struct(int curr) ltm = gmtime((const time_t *) &record_hdr[curr].ust_time); } - rectime = *ltm; + if (ltm) { + rectime = *ltm; + } } /* diff --git a/sar.c b/sar.c index 9fcc0f7..69b9709 100644 --- a/sar.c +++ b/sar.c @@ -245,17 +245,20 @@ void reverse_check_act(unsigned int act_nr) /* *************************************************************************** - * Fill the rectime structure with current record's time, based on current - * record's time data saved in file. + * Fill the (struct tm) rectime structure with current record's time, + * based on current record's time data saved in file. * The resulting timestamp is expressed in the locale of the file creator * or in the user's own locale depending on whether option -t has been used * or not. * * IN: * @curr Index in array for current sample statistics. + * + * RETURNS: + * 1 if an error was detected, or 0 otherwise. *************************************************************************** */ -void sar_get_record_timestamp_struct(int curr) +int sar_get_record_timestamp_struct(int curr) { struct tm *ltm; @@ -267,9 +270,17 @@ void sar_get_record_timestamp_struct(int curr) rectime.tm_sec = record_hdr[curr].second; } else { - ltm = localtime((const time_t *) &record_hdr[curr].ust_time); + if ((ltm = localtime((const time_t *) &record_hdr[curr].ust_time)) == NULL) + /* + * An error was detected. + * The rectime structure has NOT been updated. + */ + return 1; + rectime = *ltm; } + + return 0; } /* @@ -318,15 +329,22 @@ int check_line_hdr(void) * * OUT: * @cur_time Timestamp string. + * + * RETURNS: + * 1 if an error was detected, or 0 otherwise. *************************************************************************** */ -void set_record_timestamp_string(int curr, char *cur_time, int len) +int set_record_timestamp_string(int curr, char *cur_time, int len) { /* Fill timestamp structure */ - sar_get_record_timestamp_struct(curr); + if (sar_get_record_timestamp_struct(curr)) + /* Error detected */ + return 1; /* Set cur_time date value */ strftime(cur_time, len, "%X", &rectime); + + return 0; } /* @@ -408,7 +426,7 @@ void write_stats_avg(int curr, int read_from_file, unsigned int act_id) * @cnt Number of remaining lines to display. * * RETURNS: - * 1 if stats have been successfully displayed. + * 1 if stats have been successfully displayed, and 0 otherwise. *************************************************************************** */ int write_stats(int curr, int read_from_file, long *cnt, int use_tm_start, @@ -431,9 +449,11 @@ int write_stats(int curr, int read_from_file, long *cnt, int use_tm_start, } /* Set previous timestamp */ - set_record_timestamp_string(!curr, timestamp[!curr], 16); + if (set_record_timestamp_string(!curr, timestamp[!curr], 16)) + return 0; /* Set current timestamp */ - set_record_timestamp_string(curr, timestamp[curr], 16); + if (set_record_timestamp_string(curr, timestamp[curr], 16)) + return 0; /* Check if we are beginning a new day */ if (use_tm_start && record_hdr[!curr].ust_time && @@ -571,7 +591,7 @@ int sa_read(void *buffer, int size) * @ifd Input file descriptor. * * RETURNS: - * 1 if the record has been successfully displayed. + * 1 if the record has been successfully displayed, and 0 otherwise. *************************************************************************** */ int sar_print_special(int curr, int use_tm_start, int use_tm_end, int rtype, int ifd) @@ -579,7 +599,8 @@ int sar_print_special(int curr, int use_tm_start, int use_tm_end, int rtype, int char cur_time[26]; int dp = 1; - set_record_timestamp_string(curr, cur_time, 26); + if (set_record_timestamp_string(curr, cur_time, 26)) + return 0; /* The record must be in the interval specified by -s/-e options */ if ((use_tm_start && (datecmp(&rectime, &tm_start) < 0)) || @@ -879,7 +900,12 @@ void read_stats_from_file(char from_file[]) */ read_file_stat_bunch(act, 0, ifd, file_hdr.sa_nr_act, file_actlst); - sar_get_record_timestamp_struct(0); + if (sar_get_record_timestamp_struct(0)) + /* + * An error was detected. + * The timestamp hasn't been updated. + */ + continue; } } while ((rtype == R_RESTART) || (rtype == R_COMMENT) || -- 2.40.0