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 <sysstat@users.noreply.github.com>

diff --git a/sa_common.c b/sa_common.c
index 97265299651135d56015fc53048d13b3afbddf7e..1cdb3e58f50d947fd829cceb180dc9c507c14503 100644 (file)
--- 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 */