From 5d25419188d39e4c69c0fd874921e639c4c2cc38 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Wed, 14 Nov 2007 10:53:05 +0000 Subject: [PATCH] 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 --- ChangeLog | 18 ++++++++++++++---- qemud/qemud.c | 16 ++++++---------- src/internal.h | 19 +++++++++++++++++++ src/nodeinfo.c | 24 +++++++++++++++++++----- 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/ChangeLog b/ChangeLog index 0c145e66eb..1923a023fa 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,12 +1,22 @@ Wed Nov 14 11:34:35 CET 2007 Jim Meyering + 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 8e3ba546a3..ee6f6ce4e4 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 4a31ea6c96..f6fb1c5d3c 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 01d7e28e66..a0a26eb16b 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) {