mirror of
https://github.com/xen-project/xen.git
synced 2026-10-28 02:01:24 +01:00
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@chilopoda.uk.xensource.com/ Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com> Reviewed-by: Anthony PERARD <anthony.perard@vates.tech>
This commit is contained in:
1 parent
0c53660c1c
commit
292fb23a92
2 files changed
+28
-4
No files matched your search
@@ -27,7 +27,7 @@ output version information and exit
|
|||||||
|
|
||||||
=item B<-d>, B<--delay>=I<SECONDS>
|
=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>
|
=item B<-n>, B<--networks>
|
||||||
|
|
||||||
@@ -55,7 +55,8 @@ output data in batch mode (to stdout)
|
|||||||
|
|
||||||
=item B<-i>, B<--iterations>=I<ITERATIONS>
|
=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>
|
=item B<-z>, B<--dom0-first>
|
||||||
|
|
||||||
|
|||||||
+25
-2
@@ -23,6 +23,7 @@
|
|||||||
|
|
||||||
#include <ctype.h>
|
#include <ctype.h>
|
||||||
#include <errno.h>
|
#include <errno.h>
|
||||||
|
#include <limits.h>
|
||||||
#include <math.h>
|
#include <math.h>
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include <stdlib.h>
|
#include <stdlib.h>
|
||||||
@@ -1297,6 +1298,28 @@ static void signal_exit_handler(int sig)
|
|||||||
signal_exit = 1;
|
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 main(int argc, char **argv)
|
||||||
{
|
{
|
||||||
int opt, optind = 0;
|
int opt, optind = 0;
|
||||||
@@ -1347,7 +1370,7 @@ int main(int argc, char **argv)
|
|||||||
show_vcpus = 1;
|
show_vcpus = 1;
|
||||||
break;
|
break;
|
||||||
case 'd':
|
case 'd':
|
||||||
delay = atoi(optarg);
|
delay = parse_uint_arg("--delay", optarg, 0);
|
||||||
break;
|
break;
|
||||||
case 'b':
|
case 'b':
|
||||||
batch = 1;
|
batch = 1;
|
||||||
@@ -1356,7 +1379,7 @@ int main(int argc, char **argv)
|
|||||||
show_pcpus = 1;
|
show_pcpus = 1;
|
||||||
break;
|
break;
|
||||||
case 'i':
|
case 'i':
|
||||||
iterations = atoi(optarg);
|
iterations = parse_uint_arg("--iterations", optarg, 1);
|
||||||
loop = 0;
|
loop = 0;
|
||||||
break;
|
break;
|
||||||
case 'f':
|
case 'f':
|
||||||
|
|||||||