commit 5ebdf2fe3f1aca661ed1a13a6aa45e3c27d228c2

Author: Nathan Scott <nathans@redhat.com>

Date:   Thu Feb 13 14:12:49 2014 +1100

    Fix valgrind errors in pmdalinux /proc/stat refresh

    This one has bugged me for a long time, and I finally needed to spend

    some time hunting an obscure memory corruption in pmdalinux - so got

    to revisit this properly.

    There were a few small problems in refresh_proc_stat causing valgrind

    to report "invalid read of zero size".  Most problematic was the code

    reading the stat buffer initially, which slightly confuses the offset

    and size combo in passing a buffer pointer to read(2).  The realloc

    call here also happened to allocate 8x the (small) memory it wanted,

    accidently multiplying the size by sizeof(char).

    The nbufindex calculation also ended up with one more line than was

    in the buffer, which was OK because it always created an empty line

    at the end of it (which doesn't actually exist in /proc/stat).

    Finally, added some small improvements to the handling of ENOMEM for

    these global stat buffers, and removed a double-memset on the initial

    call (which callocs a zeroed buffer).

diff --git a/src/pmdas/linux/proc_stat.c b/src/pmdas/linux/proc_stat.c

index f4e001c..234b713 100644

--- a/src/pmdas/linux/proc_stat.c

+++ b/src/pmdas/linux/proc_stat.c

@@ -1,7 +1,7 @@

 /*

  * Linux /proc/stat metrics cluster

  *

- * Copyright (c) 2012 Red Hat.

+ * Copyright (c) 2012-2014 Red Hat.

  * Copyright (c) 2008-2009 Aconex.  All Rights Reserved.

  * Copyright (c) 2000,2004-2008 Silicon Graphics, Inc.  All Rights Reserved.

  * 

@@ -39,22 +39,28 @@ refresh_proc_stat(proc_cpuinfo_t *proc_cpuinfo, proc_stat_t *proc_stat)

     static char **bufindex;

     static int nbufindex;

     static int maxbufindex;

+    int size;

     int n;

     int i;

     int j;

-    if (fd >= 0)

-    	lseek(fd, 0, SEEK_SET);

-    else

+    if (fd >= 0) {

+	if (lseek(fd, 0, SEEK_SET) < 0)

+	    return -oserror();

+    } else {

 	if ((fd = open("/proc/stat", O_RDONLY)) < 0)

 	    return -oserror();

+    }

     for (n=0;;) {

-	if (n >= maxstatbuf) {

-	    maxstatbuf += 512;

-	    statbuf = (char *)realloc(statbuf, maxstatbuf * sizeof(char));

+	while (n >= maxstatbuf) {

+	    size = maxstatbuf + 512;

+	    if ((statbuf = (char *)realloc(statbuf, size)) == NULL)

+		return -ENOMEM;

+	    maxstatbuf = size;

 	}

-	if ((i = read(fd, statbuf+n, 512)) > 0)

+	size = (statbuf + maxstatbuf) - (statbuf + n);

+	if ((i = read(fd, statbuf + n, size)) > 0)

 	    n += i;

 	else

 	    break;

@@ -62,20 +68,24 @@ refresh_proc_stat(proc_cpuinfo_t *proc_cpuinfo, proc_stat_t *proc_stat)

     statbuf[n] = '\0';

     if (bufindex == NULL) {

+	size = 4 * sizeof(char *);

+	if ((bufindex = (char **)malloc(size)) == NULL)

+	    return -ENOMEM;

 	maxbufindex = 4;

-	bufindex = (char **)malloc(maxbufindex * sizeof(char *));

     }

     nbufindex = 0;

-    bufindex[nbufindex++] = statbuf;

+    bufindex[nbufindex] = statbuf;

     for (i=0; i < n; i++) {

-	if (statbuf[i] == '\n') {

+	if (statbuf[i] == '\n' || statbuf[i] == '\0') {

 	    statbuf[i] = '\0';

-	    if (nbufindex >= maxbufindex) {

+	    if (nbufindex + 1 >= maxbufindex) {

+		size = (maxbufindex + 4) * sizeof(char *);

+		if ((bufindex = (char **)realloc(bufindex, size)) == NULL)

+		    return -ENOMEM;

 	    	maxbufindex += 4;

-		bufindex = (char **)realloc(bufindex, maxbufindex * sizeof(char *));

 	    }

-	    bufindex[nbufindex++] = statbuf + i + 1;

+	    bufindex[++nbufindex] = statbuf + i + 1;

 	}

     }

@@ -130,19 +140,19 @@ refresh_proc_stat(proc_cpuinfo_t *proc_cpuinfo, proc_stat_t *proc_stat)

 	proc_stat->n_steal = calloc(1, n);

 	proc_stat->n_guest = calloc(1, n);

     }

-

-    /* reset per-node stats */

-    n = proc_cpuinfo->node_indom->it_numinst * sizeof(unsigned long long);

-    memset(proc_stat->n_user, 0, n);

-    memset(proc_stat->n_nice, 0, n);

-    memset(proc_stat->n_sys, 0, n);

-    memset(proc_stat->n_idle, 0, n);

-    memset(proc_stat->n_wait, 0, n);

-    memset(proc_stat->n_irq, 0, n);

-    memset(proc_stat->n_sirq, 0, n);

-    memset(proc_stat->n_steal, 0, n);

-    memset(proc_stat->n_guest, 0, n);

-

+    else {

+	/* reset per-node stats */

+	n = proc_cpuinfo->node_indom->it_numinst * sizeof(unsigned long long);

+	memset(proc_stat->n_user, 0, n);

+	memset(proc_stat->n_nice, 0, n);

+	memset(proc_stat->n_sys, 0, n);

+	memset(proc_stat->n_idle, 0, n);

+	memset(proc_stat->n_wait, 0, n);

+	memset(proc_stat->n_irq, 0, n);

+	memset(proc_stat->n_sirq, 0, n);

+	memset(proc_stat->n_steal, 0, n);

+	memset(proc_stat->n_guest, 0, n);

+    }

     /*

      * cpu  95379 4 20053 6502503

      * 2.6 kernels have 3 additional fields

commit f5eaeea7900390ae007f5ed3375cadc6ca86497e

Author: Nathan Scott <nathans@redhat.com>

Date:   Thu Feb 13 14:20:52 2014 +1100

    Fix s390x platform issues in /proc/cpuinfo parser

    The format of /proc/cpuinfo on s390x is nothing like any other

    platform.  We cannot attempt to push it through the same code

    path, it simply has nothing in common.  If we try (as we used to)

    things fall apart when we use the (negative) cpu number local

    variable to index into the cpuinfo array.

    Resolved by adding in some defensive code to short-circuit the

    decoding logic when an unexpected file format is seen.

diff --git a/src/pmdas/linux/proc_cpuinfo.c b/src/pmdas/linux/proc_cpuinfo.c

index 6380bc2..d0aa52a 100644

--- a/src/pmdas/linux/proc_cpuinfo.c

+++ b/src/pmdas/linux/proc_cpuinfo.c

@@ -1,7 +1,7 @@

 /*

  * Linux /proc/cpuinfo metrics cluster

  *

- * Copyright (c) 2013 Red Hat.

+ * Copyright (c) 2013-2014 Red Hat.

  * Copyright (c) 2000-2005 Silicon Graphics, Inc.  All Rights Reserved.

  * Portions Copyright (c) 2001 Gilly Ran (gilly@exanet.com) - for the

  * portions supporting the Alpha platform.  All rights reserved.

@@ -185,6 +185,9 @@ refresh_proc_cpuinfo(proc_cpuinfo_t *proc_cpuinfo)

 	}

 #endif

+	if (cpunum < 0 || cpunum >= proc_cpuinfo->cpuindom->it_numinst)

+	    continue;

+

 	info = &proc_cpuinfo->cpuinfo[cpunum];

 	/* note: order is important due to strNcmp comparisons */