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(a)redhat.com&gt; --- 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