From: Sebastien GODARD Date: Sun, 15 Dec 2019 17:38:49 +0000 (+0100) Subject: Fix #243: Heap overflow in logic2_display_loop() X-Git-Tag: v12.3.1~8 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=763d8ddeebbcdf92c05b903908a60a04ae1b260c;p=sysstat Fix #243: Heap overflow in logic2_display_loop() Make sure that all tests from check_file_actlst() function are done on a data file, even if this file has activities with unknown formats. "sadf -H" and, e.g., "sadf -Hx" should as a consequence behave the same. Signed-off-by: Sebastien GODARD --- diff --git a/format.c b/format.c index 77717aa..84521b9 100644 --- a/format.c +++ b/format.c @@ -40,7 +40,7 @@ */ struct report_format hdr_fmt = { .id = F_HEADER_OUTPUT, - .options = FO_HEADER_ONLY + FO_BAD_FILE_FORMAT, + .options = FO_HEADER_ONLY, .f_header = print_hdr_header, .f_statistics = NULL, .f_timestamp = NULL, @@ -112,7 +112,7 @@ struct report_format json_fmt = { */ struct report_format conv_fmt = { .id = F_CONV_OUTPUT, - .options = FO_BAD_FILE_FORMAT, + .options = 0, .f_header = NULL, .f_statistics = NULL, .f_timestamp = NULL, diff --git a/sa.h b/sa.h index 3c3a0c7..615b1f4 100644 --- a/sa.h +++ b/sa.h @@ -1446,7 +1446,7 @@ int check_disk_reg (struct activity *, int, int, int); void check_file_actlst (int *, char *, struct activity * [], uint64_t, struct file_magic *, - struct file_header *, struct file_activity **, unsigned int [], int, int *, int *); + struct file_header *, struct file_activity **, unsigned int [], int *, int *); int check_net_dev_reg (struct activity *, int, int, int); int check_net_edev_reg diff --git a/sa_common.c b/sa_common.c index 856a371..87e1e0f 100644 --- a/sa_common.c +++ b/sa_common.c @@ -1922,9 +1922,6 @@ int sa_open_read_magic(int *fd, char *dfile, struct file_magic *file_magic, * @dfile Name of system activity data file. * @act Array of activities. * @flags Flags for common options and system state. - * @ignore Set to 1 if a true sysstat activity file but with a bad - * format should not yield an error message. Used with - * sadf -H (sadf -c doesn't call check_file_actlst() function). * * OUT: * @ifd System activity data file descriptor. @@ -1942,16 +1939,17 @@ int sa_open_read_magic(int *fd, char *dfile, struct file_magic *file_magic, void check_file_actlst(int *ifd, char *dfile, struct activity *act[], uint64_t flags, struct file_magic *file_magic, struct file_header *file_hdr, struct file_activity **file_actlst, unsigned int id_seq[], - int ignore, int *endian_mismatch, int *arch_64) + int *endian_mismatch, int *arch_64) { - int i, j, k, p; + int i, j, k, p, skip; struct file_activity *fal; void *buffer = NULL; size_t bh_size = FILE_HEADER_SIZE; size_t ba_size = FILE_ACTIVITY_SIZE; /* Open sa data file and read its magic structure */ - if (sa_open_read_magic(ifd, dfile, file_magic, ignore, endian_mismatch, TRUE) < 0) + if (sa_open_read_magic(ifd, dfile, file_magic, + DISPLAY_HDR_ONLY(flags), endian_mismatch, TRUE) < 0) /* * Not current sysstat's format. * Return now so that sadf -H can display at least @@ -2077,17 +2075,19 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], uint64_t f /* Unknown activity */ continue; - if (act[p]->magic != fal->magic) { + skip = FALSE; + if (fal->magic != act[p]->magic) { /* Bad magical number */ - if (ignore) { + if (DISPLAY_HDR_ONLY(flags)) { /* * This is how sadf -H knows that this * activity has an unknown format. */ act[p]->magic = ACTIVITY_MAGIC_UNKNOWN; } - else - continue; + else { + skip = TRUE; + } } /* Check max value for known activities */ @@ -2112,7 +2112,16 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], uint64_t f || ((fal->types_nr[0] <= act[p]->gtypes_nr[0]) && (fal->types_nr[1] <= act[p]->gtypes_nr[1]) && - (fal->types_nr[2] <= act[p]->gtypes_nr[2]))) && !ignore) { + (fal->types_nr[2] <= act[p]->gtypes_nr[2]))) && + (act[p]->magic != ACTIVITY_MAGIC_UNKNOWN) && !DISPLAY_HDR_ONLY(flags)) { + /* + * This may not be an error (that's actually why we may have changed + * the magic number for this activity above). + * So, if the activity magic number has changed (e.g.: ACTIVITY_MAGIC_UNKNOWN) + * and we want to display only the header, then ignore the error. + * If we want to also display the stats then we must stop here because + * we won't know how to map the contents of the stats structure. + */ #ifdef DEBUG fprintf(stderr, "%s: id=%d file=%d,%d,%d activity=%d,%d,%d\n", __FUNCTION__, fal->id, fal->types_nr[0], fal->types_nr[1], fal->types_nr[2], @@ -2129,6 +2138,13 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], uint64_t f goto format_error; } + if (skip) + /* + * This is an unknown activity and we want stats about it: + * This is not possible so skip it. + */ + continue; + for (k = 0; k < 3; k++) { act[p]->ftypes_nr[k] = fal->types_nr[k]; } diff --git a/sadf.c b/sadf.c index a931b09..34e317b 100644 --- a/sadf.c +++ b/sadf.c @@ -1446,12 +1446,11 @@ void read_stats_from_file(char dfile[], char pcparchive[]) struct file_magic file_magic; struct file_activity *file_actlst = NULL; struct tm rectime; - int ifd, ignore, tab = 0; + int ifd, tab = 0; /* Prepare file for reading and read its headers */ - ignore = ACCEPT_BAD_FILE_FORMAT(fmt[f_position]->options); check_file_actlst(&ifd, dfile, act, flags, &file_magic, &file_hdr, - &file_actlst, id_seq, ignore, &endian_mismatch, &arch_64); + &file_actlst, id_seq, &endian_mismatch, &arch_64); if (DISPLAY_HDR_ONLY(flags)) { if (*fmt[f_position]->f_header) { diff --git a/sadf.h b/sadf.h index c845c4d..60eaa9f 100644 --- a/sadf.h +++ b/sadf.h @@ -54,11 +54,7 @@ */ #define FO_HEADER_ONLY 0x02 -/* - * Indicate that a true sysstat activity file but with a bad - * format should not yield an error message. - */ -#define FO_BAD_FILE_FORMAT 0x04 +/* Unused: 0x04 */ /* * Indicate that timestamp can be displayed in local time instead of UTC @@ -110,7 +106,6 @@ #define SET_LC_NUMERIC_C(m) (((m) & FO_LC_NUMERIC_C) == FO_LC_NUMERIC_C) #define ACCEPT_HEADER_ONLY(m) (((m) & FO_HEADER_ONLY) == FO_HEADER_ONLY) -#define ACCEPT_BAD_FILE_FORMAT(m) (((m) & FO_BAD_FILE_FORMAT) == FO_BAD_FILE_FORMAT) #define ACCEPT_LOCAL_TIME(m) (((m) & FO_LOCAL_TIME) == FO_LOCAL_TIME) #define ACCEPT_HORIZONTALLY(m) (((m) & FO_HORIZONTALLY) == FO_HORIZONTALLY) #define ACCEPT_SEC_EPOCH(m) (((m) & FO_SEC_EPOCH) == FO_SEC_EPOCH) diff --git a/sar.c b/sar.c index d946a49..9eeeeb2 100644 --- a/sar.c +++ b/sar.c @@ -1001,7 +1001,7 @@ void read_stats_from_file(char from_file[]) /* Read file headers and activity list */ check_file_actlst(&ifd, from_file, act, flags, &file_magic, &file_hdr, - &file_actlst, id_seq, FALSE, &endian_mismatch, &arch_64); + &file_actlst, id_seq, &endian_mismatch, &arch_64); /* Perform required allocations */ allocate_structures(act);