From ad4fe0fef5f4a8601e29939bc85093c0f7d6f1b9 Mon Sep 17 00:00:00 2001 From: Sebastien GODARD Date: Sun, 24 Sep 2017 18:16:35 +0200 Subject: [PATCH] Fortify use of structures map Add extra checks on *_types_nr[] values read from a file to cope with possible corrupted files. *_types_nr[] values give the number of fields of each type (long long, long, int) composing a structure. Signed-off-by: Sebastien GODARD --- rd_stats.h | 4 ++++ sa.h | 6 +++--- sa_common.c | 16 ++++++++++++---- sadf.c | 2 +- sar.c | 2 +- svg_stats.c | 8 ++++---- 6 files changed, 25 insertions(+), 13 deletions(-) diff --git a/rd_stats.h b/rd_stats.h index f3eda2f..69a9f90 100644 --- a/rd_stats.h +++ b/rd_stats.h @@ -83,6 +83,10 @@ #define UL_ALIGNMENT_WIDTH SIZEOF_LONG_64BIT #define U_ALIGNMENT_WIDTH 4 +#define MAP_SIZE(m) ((m[0] * ULL_ALIGNMENT_WIDTH) + \ + (m[1] * UL_ALIGNMENT_WIDTH) + \ + (m[2] * U_ALIGNMENT_WIDTH)) + /* * Structure for CPU statistics. * In activity buffer: First structure is for global CPU utilisation ("all"). diff --git a/sa.h b/sa.h index fdb564b..296ec45 100644 --- a/sa.h +++ b/sa.h @@ -509,7 +509,7 @@ struct file_activity { * Description of the structure containing statistics for the * given activity (nr of "long long", nr of "long" and nr of "int"). */ - int types_nr[3]; + unsigned int types_nr[3]; }; #define FILE_ACTIVITY_SIZE (sizeof(struct file_activity)) @@ -756,13 +756,13 @@ struct activity { * gives the number of "long long" fields composing the structure, then the number * of "long" fields, then the number of "int" fields. */ - int gtypes_nr[3]; + unsigned int gtypes_nr[3]; /* * This array has the same meaning as @gtypes_nr[] above, but the values are those * read from current data file. They may be different from those of @gtypes_nr[] * because we can read data from a different sysstat version (older or newer). */ - int ftypes_nr[3]; + unsigned int ftypes_nr[3]; /* * Number of SVG graphs for this activity. The total number of graphs for * the activity can be greater though if flag AO_GRAPH_PER_ITEM is set, in diff --git a/sa_common.c b/sa_common.c index 5a7493a..2b84e4b 100644 --- a/sa_common.c +++ b/sa_common.c @@ -50,8 +50,8 @@ int default_file_used = FALSE; extern struct act_bitmap cpu_bitmap; -int hdr_types_nr[] = {FILE_HEADER_ULL_NR, FILE_HEADER_UL_NR, FILE_HEADER_U_NR}; -int act_types_nr[] = {FILE_ACTIVITY_ULL_NR, FILE_ACTIVITY_UL_NR, FILE_ACTIVITY_U_NR}; +unsigned int hdr_types_nr[] = {FILE_HEADER_ULL_NR, FILE_HEADER_UL_NR, FILE_HEADER_U_NR}; +unsigned int act_types_nr[] = {FILE_ACTIVITY_ULL_NR, FILE_ACTIVITY_UL_NR, FILE_ACTIVITY_U_NR}; /* *************************************************************************** @@ -993,7 +993,7 @@ void select_default_activity(struct activity *act[]) * @is64bit TRUE if data come from a 64-bit machine. *************************************************************************** */ -void swap_struct(int types_nr[], void *ps, int is64bit) +void swap_struct(unsigned int types_nr[], void *ps, int is64bit) { int i; uint64_t *x; @@ -1049,10 +1049,15 @@ void swap_struct(int types_nr[], void *ps, int is64bit) * structure expected by current sysstat version). *************************************************************************** */ -void remap_struct(int gtypes_nr[], int ftypes_nr[], void *ps, int st_size) +void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[], + void *ps, unsigned int st_size) { int d; + /* Sanity check */ + if (MAP_SIZE(ftypes_nr) > st_size) + return; + /* Remap [unsigned] long fields */ d = gtypes_nr[0] - ftypes_nr[0]; if (d) { @@ -1521,6 +1526,9 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], (fal->types_nr[2] <= act[p]->gtypes_nr[2])))) { handle_invalid_sa_file(ifd, file_magic, dfile, 0); } + if (MAP_SIZE(fal->types_nr) > fal->size) { + handle_invalid_sa_file(ifd, file_magic, dfile, 0); + } for (k = 0; k < 3; k++) { act[p]->ftypes_nr[k] = fal->types_nr[k]; } diff --git a/sadf.c b/sadf.c index 3de2f70..473e155 100644 --- a/sadf.c +++ b/sadf.c @@ -53,7 +53,7 @@ int endian_mismatch = FALSE; /* TRUE if file's data come from a 64 bit machine */ int arch_64 = FALSE; -int rec_types_nr[] = {RECORD_HEADER_ULL_NR, RECORD_HEADER_UL_NR, RECORD_HEADER_U_NR}; +unsigned int rec_types_nr[] = {RECORD_HEADER_ULL_NR, RECORD_HEADER_UL_NR, RECORD_HEADER_U_NR}; unsigned int flags = 0; unsigned int dm_major; /* Device-mapper major number */ diff --git a/sar.c b/sar.c index fb42b0f..a110a51 100644 --- a/sar.c +++ b/sar.c @@ -61,7 +61,7 @@ unsigned int flags = 0; unsigned int dm_major; /* Device-mapper major number */ char timestamp[2][TIMESTAMP_LEN]; -int rec_types_nr[] = {RECORD_HEADER_ULL_NR, RECORD_HEADER_UL_NR, RECORD_HEADER_U_NR}; +unsigned int rec_types_nr[] = {RECORD_HEADER_ULL_NR, RECORD_HEADER_UL_NR, RECORD_HEADER_U_NR}; unsigned long avg_count = 0; diff --git a/svg_stats.c b/svg_stats.c index d89733b..2444ed1 100644 --- a/svg_stats.c +++ b/svg_stats.c @@ -75,7 +75,7 @@ unsigned int svg_colors[] = {0x00cc00, 0xff00bf, 0x00ffff, 0xff0000, * @spmax Array containg the possible new max values for current activity. *************************************************************************** */ -void save_extrema(int types_nr[], void *cs, void *ps, unsigned long long itv, +void save_extrema(unsigned int types_nr[], void *cs, void *ps, unsigned long long itv, double *spmin, double *spmax, int g_fields[]) { unsigned long long *lluc, *llup; @@ -1966,7 +1966,7 @@ __print_funct_t svg_print_disk_stats(struct activity *a, int curr, int action, s "await", "svctm", "%util"}; int g_fields[] = {0, 1, 2}; - int local_types_nr[] = {1, 0, 0}; + unsigned int local_types_nr[] = {1, 0, 0}; static double *spmin, *spmax; static char **out; static int *outsize; @@ -2218,7 +2218,7 @@ __print_funct_t svg_print_net_dev_stats(struct activity *a, int curr, int action "rxcmp/s", "txcmp/s", "rxmcst/s", "%ifutil"}; int g_fields[] = {0, 1, 2, 3, 4, 5, 6}; - int local_types_nr[] = {7, 0, 0}; + unsigned int local_types_nr[] = {7, 0, 0}; static double *spmin, *spmax; static char **out; static int *outsize; @@ -4432,7 +4432,7 @@ __print_funct_t svg_print_huge_stats(struct activity *a, int curr, int action, s char *g_title[] = {"~kbhugfree", "~kbhugused", "%hugused"}; int g_fields[] = {0}; - int local_types_nr[] = {0, 1, 0}; + unsigned int local_types_nr[] = {0, 1, 0}; static double *spmin, *spmax; static char **out; static int *outsize; -- 2.40.0