Parse integers more carefully, cont'd.

* qemud/qemud.c: Replace uses of strtol with uses of xstrtol_i.
  Avoid overflow for very large --timeout=N values.
* src/nodeinfo.c: In linuxNodeInfoMemPopulate and
  linuxNodeInfoCPUPopulate, use xstrtol_i rather than strtol.
  Unlike in qemud.c, here we allow trailing "isspace", and in
  the case of "cpuinfo cpu MHz", also allow a "." terminator,
  since we ignore the decimal and any following digits.
* src/internal.h: Define xstrtol_ui, too.

Author: Jim Meyering <meyering@redhat.com>

diff --git a/ChangeLog b/ChangeLog
index 0c145e66eb611355df0b266d156751c8397421bd..1923a023fadb1b78786e34639f5007d0fb47bda8 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,12 +1,22 @@
 Wed Nov 14 11:34:35 CET 2007 Jim Meyering <meyering@redhat.com>
 
+       Parse integers more carefully, cont'd.
+       * qemud/qemud.c: Replace uses of strtol with uses of xstrtol_i.
+         Avoid overflow for very large --timeout=N values.
+       * src/nodeinfo.c: In linuxNodeInfoMemPopulate and
+         linuxNodeInfoCPUPopulate, use xstrtol_i rather than strtol.
+         Unlike in qemud.c, here we allow trailing "isspace", and in
+         the case of "cpuinfo cpu MHz", also allow a "." terminator,
+         since we ignore the decimal and any following digits.
+       * src/internal.h: Define xstrtol_ui, too.
+
        Arrange for tests to pass in a non-srcdir build.
        * tests/Makefile.am: Include the contents of the *data directories
-       in the make-dist-built tarball by adding each of that *data
-       directories to EXTRA_DIST.
-       Also add int-overflow (via $(test_scripts)) to EXTRA_DIST.
+         in the make-dist-built tarball by adding each of that *data
+         directories to EXTRA_DIST.
+         Also add int-overflow (via $(test_scripts)) to EXTRA_DIST.
        * tests/nodeinfotest.c: Prepend "$abs_top_srcdir/tests" to
-       each input file name.
+         each input file name.
        * tests/qemuxml2argvtest.c: Likewise.
        * tests/qemuxml2xmltest.c: Likewise.
        * tests/sexpr2xmltest.c: Likewise.

diff --git a/qemud/qemud.c b/qemud/qemud.c
index 8e3ba546a34917da37189d97b2db021991685f76..ee6f6ce4e4ddb3a9cb30b93bc2bb25877831f439 100644 (file)
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -1583,9 +1583,7 @@ remoteReadConfigFile (const char *filename)
     p = virConfGetValue (conf, "unix_sock_ro_perms");
     CHECK_TYPE ("unix_sock_ro_perms", VIR_CONF_STRING);
     if (p && p->str) {
-        char *tmp = NULL;
-        unix_sock_ro_perms = strtol(p->str, &tmp, 8);
-        if (*tmp) {
+        if (xstrtol_i(p->str, NULL, 8, &unix_sock_ro_perms) != 0) {
             qemudLog (QEMUD_ERR, "Failed to parse mode '%s'", p->str);
             return -1;
         }
@@ -1594,9 +1592,7 @@ remoteReadConfigFile (const char *filename)
     p = virConfGetValue (conf, "unix_sock_rw_perms");
     CHECK_TYPE ("unix_sock_rw_perms", VIR_CONF_STRING);
     if (p && p->str) {
-        char *tmp = NULL;
-        unix_sock_rw_perms = strtol(p->str, &tmp, 8);
-        if (*tmp) {
+        if (xstrtol_i(p->str, NULL, 8, &unix_sock_rw_perms) != 0) {
             qemudLog (QEMUD_ERR, "Failed to parse mode '%s'", p->str);
             return -1;
         }
@@ -1797,10 +1793,10 @@ int main(int argc, char **argv) {
             break;
 
         case 't':
-            timeout = strtol(optarg, &tmp, 10);
-            if (!tmp)
-                timeout = -1;
-            if (timeout <= 0)
+            if (xstrtol_i(optarg, &tmp, 10, &timeout) != 0
+                || timeout <= 0
+                /* Ensure that we can multiply by 1000 without overflowing.  */
+                || timeout > INT_MAX / 1000)
                 timeout = -1;
             break;
 

diff --git a/src/internal.h b/src/internal.h
index 4a31ea6c96bc966a6dadc54e131afacce62c4119..f6fb1c5d3c1b4426ea16d80970a86f7687d7c67b 100644 (file)
--- a/src/internal.h
+++ b/src/internal.h
@@ -264,6 +264,25 @@ xstrtol_i(char const *s, char **end_ptr, int base, int *result)
     return 0;
 }
 
+/* Just like xstrtol_i, above, but produce an "unsigned int" value.  */
+static inline int
+xstrtol_ui(char const *s, char **end_ptr, int base, unsigned int *result)
+{
+    unsigned long int val;
+    char *p;
+    int err;
+
+    errno = 0;
+    val = strtoul(s, &p, base);
+    err = (errno || (!end_ptr && *p) || p == s || (unsigned int) val != val);
+    if (end_ptr)
+        *end_ptr = p;
+    if (err)
+        return -1;
+    *result = val;
+    return 0;
+}
+
 #ifdef __cplusplus
 }
 #endif                          /* __cplusplus */

diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 01d7e28e66cf89edf4d5c6d2556f9dc26c4a4651..a0a26eb16bf7798420fbc5f12b70fa188e6bd30f 100644 (file)
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -63,6 +63,8 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n
             }
             nodeinfo->cpus++;
         } else if (STREQLEN(buf, "cpu MHz", 7)) {
+            char *p;
+            unsigned int ui;
             buf += 9;
             while (*buf && isspace(*buf))
                 buf++;
@@ -72,8 +74,12 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n
                                 "parsing cpuinfo cpu MHz");
                 return -1;
             }
-            nodeinfo->mhz = (unsigned int)strtol(buf+1, NULL, 10);
+            if (xstrtol_ui(buf+1, &p, 10, &ui) == 0
+                /* Accept trailing fractional part.  */
+                && (*p == '\0' || *p == '.' || isspace(*p)))
+                nodeinfo->mhz = ui;
         } else if (STREQLEN(buf, "cpu cores", 9)) { /* aka cores */
+            char *p;
             unsigned int id;
             buf += 9;
             while (*buf && isspace(*buf))
@@ -84,8 +90,9 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n
                                 "parsing cpuinfo cpu cores %c", *buf);
                 return -1;
             }
-            id = (unsigned int)strtol(buf+1, NULL, 10);
-            if (id > nodeinfo->cores)
+            if (xstrtol_ui(buf+1, &p, 10, &id) == 0
+                && (*p == '\0' || isspace(*p))
+                && id > nodeinfo->cores)
                 nodeinfo->cores = id;
         }
     }
@@ -108,14 +115,21 @@ int linuxNodeInfoCPUPopulate(virConnectPtr conn, FILE *cpuinfo, virNodeInfoPtr n
 }
 
 
-int linuxNodeInfoMemPopulate(virConnectPtr conn, FILE *meminfo, virNodeInfoPtr nodeinfo) {
+int linuxNodeInfoMemPopulate(virConnectPtr conn, FILE *meminfo,
+                             virNodeInfoPtr nodeinfo) {
     char line[1024];
 
     nodeinfo->memory = 0;
 
     while (fgets(line, sizeof(line), meminfo) != NULL) {
         if (STREQLEN(line, "MemTotal:", 9)) {
-            nodeinfo->memory = (unsigned int)strtol(line + 10, NULL, 10);
+            char *p;
+            unsigned int ui;
+            if (xstrtol_ui(line + 10, &p, 10, &ui) == 0
+                && (*p == '\0' || isspace(*p))) {
+                nodeinfo->memory = ui;
+                break;
+            }
         }
     }
     if (!nodeinfo->memory) {