[PATCH] Parse integers more carefully, cont'd.

[Jim Meyering <jim@xxxxxxxxxxxx>]

Here's part two in the strtol fix-up series:

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_iu, too.

Signed-off-by: Jim Meyering <meyering@xxxxxxxxxx>
---
 qemud/qemud.c  |   16 ++++++----------
 src/internal.h |   19 +++++++++++++++++++
 src/nodeinfo.c |   24 +++++++++++++++++++-----
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/qemud/qemud.c b/qemud/qemud.c
index 8e3ba54..ee6f6ce 100644
--- 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 4a31ea6..f6fb1c5 100644
--- 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 01d7e28..a0a26eb 100644
--- 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) {
-- 
1.5.3.5.643.g40e25

--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list