From cb7f009e41341c687c449dd37b9c528c4878455a Mon Sep 17 00:00:00 2001 From: Sebastien GODARD Date: Sat, 25 Aug 2018 15:53:39 +0200 Subject: [PATCH] sar: Fortify remap_struct() function New metrics can be added to sar with new sysstat versions. These new metrics correspond to new fields added to existing statistics structures. For compatibility reasons, sysstat assumes that these structures (identified by their number of fields of different types: long long, long and int) can only grow with newer sysstat versions. Yet commit c2f9d16 has fixed a longstanding bug regarding a wrong size used for structure stats_huge. With this bugfix structure stats_huge has now a size smaller than before. We need to update remap_struct() function to deal with this. Else adding new fields to structure stats_huge will make sar unable to read older binary datafiles and crash: free(): invalid pointer Aborted (core dumped) Signed-off-by: Sebastien GODARD --- sa_common.c | 42 ++++++++++++++++++++++++++---------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/sa_common.c b/sa_common.c index 9726529..1cdb3e5 100644 --- a/sa_common.c +++ b/sa_common.c @@ -1267,7 +1267,7 @@ void swap_struct(unsigned int types_nr[], void *ps, int is64bit) * Map the fields of a structure containing statistics read from a file to * those of the structure known by current sysstat version. * Each structure (either read from file or from current sysstat version) - * are described by 3 values: The number of [unsigned] long long integers, + * is described by 3 values: The number of [unsigned] long long integers, * the number of [unsigned] long integers following in the structure, and * last the number of [unsigned] integers. * We assume that those numbers will *never* decrease with newer sysstat @@ -1277,18 +1277,18 @@ void swap_struct(unsigned int types_nr[], void *ps, int is64bit) * @gtypes_nr Structure description as expected for current sysstat version. * @ftypes_nr Structure description as read from file. * @ps Pointer on structure containing statistics. - * @st_size Size of the structure containing statistics. This is the size - * of the structure *read from file* (not the size of the - * structure expected by current sysstat version). + * @f_size Size of the structure containing statistics. This is the + * size of the structure *read from file*. + * @g_size Size of the structure expected by current sysstat version. *************************************************************************** */ void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[], - void *ps, unsigned int st_size) + void *ps, unsigned int f_size, unsigned int g_size) { int d; /* Sanity check */ - if (MAP_SIZE(ftypes_nr) > st_size) + if (MAP_SIZE(ftypes_nr) > f_size) return; /* Remap [unsigned] long fields */ @@ -1296,7 +1296,8 @@ void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[], if (d) { memmove(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH, ((char *) ps) + ftypes_nr[0] * ULL_ALIGNMENT_WIDTH, - st_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH); + MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH, + g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH)); if (d > 0) { memset(((char *) ps) + ftypes_nr[0] * ULL_ALIGNMENT_WIDTH, 0, d * ULL_ALIGNMENT_WIDTH); @@ -1309,8 +1310,10 @@ void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[], + gtypes_nr[1] * UL_ALIGNMENT_WIDTH, ((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + ftypes_nr[1] * UL_ALIGNMENT_WIDTH, - st_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH - - ftypes_nr[1] * UL_ALIGNMENT_WIDTH); + MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH + - ftypes_nr[1] * UL_ALIGNMENT_WIDTH, + g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + - gtypes_nr[1] * UL_ALIGNMENT_WIDTH)); if (d > 0) { memset(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + ftypes_nr[1] * UL_ALIGNMENT_WIDTH, @@ -1326,9 +1329,12 @@ void remap_struct(unsigned int gtypes_nr[], unsigned int ftypes_nr[], ((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + gtypes_nr[1] * UL_ALIGNMENT_WIDTH + ftypes_nr[2] * U_ALIGNMENT_WIDTH, - st_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH - - ftypes_nr[1] * UL_ALIGNMENT_WIDTH - - ftypes_nr[2] * U_ALIGNMENT_WIDTH); + MINIMUM(f_size - ftypes_nr[0] * ULL_ALIGNMENT_WIDTH + - ftypes_nr[1] * UL_ALIGNMENT_WIDTH + - ftypes_nr[2] * U_ALIGNMENT_WIDTH, + g_size - gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + - gtypes_nr[1] * UL_ALIGNMENT_WIDTH + - gtypes_nr[2] * U_ALIGNMENT_WIDTH)); if (d > 0) { memset(((char *) ps) + gtypes_nr[0] * ULL_ALIGNMENT_WIDTH + gtypes_nr[1] * UL_ALIGNMENT_WIDTH @@ -1406,7 +1412,8 @@ int read_record_hdr(int ifd, void *buffer, struct record_header *record_hdr, return 1; /* Remap record header structure to that expected by current version */ - remap_struct(rec_types_nr, file_hdr->rec_types_nr, buffer, file_hdr->rec_size); + remap_struct(rec_types_nr, file_hdr->rec_types_nr, buffer, + file_hdr->rec_size, RECORD_HEADER_SIZE); memcpy(record_hdr, buffer, RECORD_HEADER_SIZE); /* Normalize endianness */ @@ -1614,7 +1621,8 @@ void read_file_stat_bunch(struct activity *act[], int curr, int ifd, int act_nr, /* Remap structure's fields to those known by current sysstat version */ for (j = 0; j < (nr_value * act[p]->nr2); j++) { remap_struct(act[p]->gtypes_nr, act[p]->ftypes_nr, - (char *) act[p]->buf[curr] + j * act[p]->msize, act[p]->fsize); + (char *) act[p]->buf[curr] + j * act[p]->msize, + act[p]->fsize, act[p]->msize); } } } @@ -1795,7 +1803,8 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], * smaller than FILE_HEADER_SIZE. Remap the fields of the file header * then copy its contents to the expected structure. */ - remap_struct(hdr_types_nr, file_magic->hdr_types_nr, buffer, file_magic->header_size); + remap_struct(hdr_types_nr, file_magic->hdr_types_nr, buffer, + file_magic->header_size, FILE_HEADER_SIZE); memcpy(file_hdr, buffer, FILE_HEADER_SIZE); free(buffer); buffer = NULL; @@ -1843,7 +1852,8 @@ void check_file_actlst(int *ifd, char *dfile, struct activity *act[], * smaller than FILE_ACTIVITY_SIZE. Remap the fields of the file's structure * then copy its contents to the expected structure. */ - remap_struct(act_types_nr, file_hdr->act_types_nr, buffer, file_hdr->act_size); + remap_struct(act_types_nr, file_hdr->act_types_nr, buffer, + file_hdr->act_size, FILE_ACTIVITY_SIZE); memcpy(fal, buffer, FILE_ACTIVITY_SIZE); /* Normalize endianness for file_activity structures */ -- 2.40.0