[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[xen staging] tools/xentop: reject invalid --delay and --iterations arguments



commit 292fb23a923200fa70653d5255975c22372683e5
Author:     Matthias Goergens <matthias.goergens@xxxxxxxxx>
AuthorDate: Wed Aug 5 21:55:21 2026 +0800
Commit:     Anthony PERARD <anthony.perard@xxxxxxxxxx>
CommitDate: Mon Oct 12 17:31:39 2026 +0200

    tools/xentop: reject invalid --delay and --iterations arguments
    
    xentop parses -d/--delay and -i/--iterations with atoi(), so invalid
    input is silently accepted with surprising results: a fractional delay
    such as "-d 2.5" is truncated to 2; "-d abc" parses as 0, which in
    batch mode turns the output loop into a busy loop; "-d -1" wraps to an
    effective delay of about 136 years; and "-i 0" (or any unparsable
    iterations count) decrements an unsigned counter from zero, running
    for about 2^32 iterations.  atoi() also has undefined behaviour on
    out-of-range input.
    
    Parse both options with strtoull() instead, and reject anything that
    is not a plain decimal integer in range: a sign, a fractional part,
    trailing junk or overflow now produce an error and exit rather than a
    silently wrong value.
    
    Compatibility considerations: "--delay 0" remains accepted, since
    updating as fast as possible is a plausible deliberate choice and
    works today; "--iterations 0" is rejected, since running the loop
    2^32 times cannot be what the caller meant.  The interactive 'D'
    prompt already validates its input and is unchanged.  The only
    previously useful invocation this breaks is a fractional delay, which
    now fails loudly instead of silently rounding down - which is the
    point of the change.
    
    A patch documenting the --delay truncation was posted in 2010 but
    never applied:
    Link: 
https://lore.kernel.org/xen-devel/01ea26d2420e3562eb30.1292604768@xxxxxxxxxxxxxxxxxxxxxxxxxx/
    
    Signed-off-by: Matthias Goergens <matthias.goergens@xxxxxxxxx>
    Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
---
 docs/man/xentop.1.pod |  5 +++--
 tools/xentop/xentop.c | 27 +++++++++++++++++++++++++--
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/docs/man/xentop.1.pod b/docs/man/xentop.1.pod
index db64ceb694..13f3f13246 100644
--- a/docs/man/xentop.1.pod
+++ b/docs/man/xentop.1.pod
@@ -27,7 +27,7 @@ output version information and exit
 
 =item B<-d>, B<--delay>=I<SECONDS>
 
-seconds between updates (default 3)
+seconds between updates (default 3); must be a non-negative integer
 
 =item B<-n>, B<--networks>
 
@@ -55,7 +55,8 @@ output data in batch mode (to stdout)
 
 =item B<-i>, B<--iterations>=I<ITERATIONS>
 
-maximum number of iterations xentop should produce before ending
+maximum number of iterations xentop should produce before ending; must
+be a positive integer
 
 =item B<-z>, B<--dom0-first>
 
diff --git a/tools/xentop/xentop.c b/tools/xentop/xentop.c
index addb1c70c9..c7fb4ca13f 100644
--- a/tools/xentop/xentop.c
+++ b/tools/xentop/xentop.c
@@ -23,6 +23,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <limits.h>
 #include <math.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -1297,6 +1298,28 @@ static void signal_exit_handler(int sig)
        signal_exit = 1;
 }
 
+/* Parse a numeric command line argument as a plain decimal integer no
+ * smaller than min_val.  Anything else - a sign, a fractional part,
+ * trailing junk, overflow - is fatal, rather than being silently
+ * accepted as a wrong value the way atoi() would.
+ */
+static unsigned int parse_uint_arg(const char *name, const char *arg,
+                                  unsigned int min_val)
+{
+       unsigned long long val;
+       char *end;
+
+       errno = 0;
+       if (isdigit((unsigned char)arg[0])) {
+               val = strtoull(arg, &end, 10);
+               if (!errno && !*end && val >= min_val && val <= UINT_MAX)
+                       return val;
+       }
+       fprintf(stderr, "xentop: invalid %s argument '%s': expected a %s 
decimal integer\n",
+               name, arg, min_val ? "positive" : "non-negative");
+       exit(1);
+}
+
 int main(int argc, char **argv)
 {
        int opt, optind = 0;
@@ -1347,7 +1370,7 @@ int main(int argc, char **argv)
                        show_vcpus = 1;
                        break;
                case 'd':
-                       delay = atoi(optarg);
+                       delay = parse_uint_arg("--delay", optarg, 0);
                        break;
                case 'b':
                        batch = 1;
@@ -1356,7 +1379,7 @@ int main(int argc, char **argv)
                        show_pcpus = 1;
                        break;
                case 'i':
-                       iterations = atoi(optarg);
+                       iterations = parse_uint_arg("--iterations", optarg, 1);
                        loop = 0;
                        break;
                case 'f':
--
generated by git-patchbot for /home/xen/git/xen.git#staging



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.