From abc9bf69a66a11499a801ff545b8fe7adbb3a04c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Apr 2020 15:07:08 +0200 Subject: cpus: Fix configure_icount() error API violation The Error ** argument must be NULL, &error_abort, &error_fatal, or a pointer to a variable containing NULL. Passing an argument of the latter kind twice without clearing it in between is wrong: if the first call sets an error, it no longer points to NULL for the second call. configure_icount() is wrong that way. Harmless, because its @errp is always &error_abort or &error_fatal. Just as wrong (and just as harmless): when it fails, it can still update global state. Fix all that. Cc: Paolo Bonzini Signed-off-by: Markus Armbruster Message-Id: <20200422130719.28225-4-armbru@redhat.com> --- cpus.c | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) (limited to 'cpus.c') diff --git a/cpus.c b/cpus.c index ef441bdf62..1b542b37f9 100644 --- a/cpus.c +++ b/cpus.c @@ -797,40 +797,49 @@ void cpu_ticks_init(void) void configure_icount(QemuOpts *opts, Error **errp) { - const char *option; + const char *option = qemu_opt_get(opts, "shift"); + bool sleep = qemu_opt_get_bool(opts, "sleep", true); + bool align = qemu_opt_get_bool(opts, "align", false); + long time_shift = -1; char *rem_str = NULL; - option = qemu_opt_get(opts, "shift"); - if (!option) { - if (qemu_opt_get(opts, "align") != NULL) { - error_setg(errp, "Please specify shift option when using align"); - } + if (!option && qemu_opt_get(opts, "align")) { + error_setg(errp, "Please specify shift option when using align"); return; } - icount_sleep = qemu_opt_get_bool(opts, "sleep", true); - if (icount_sleep) { - timers_state.icount_warp_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT, - icount_timer_cb, NULL); - } - - icount_align_option = qemu_opt_get_bool(opts, "align", false); - - if (icount_align_option && !icount_sleep) { + if (align && !sleep) { error_setg(errp, "align=on and sleep=off are incompatible"); + return; } + if (strcmp(option, "auto") != 0) { errno = 0; - timers_state.icount_time_shift = strtol(option, &rem_str, 0); + time_shift = strtol(option, &rem_str, 0); if (errno != 0 || *rem_str != '\0' || !strlen(option)) { error_setg(errp, "icount: Invalid shift value"); + return; } - use_icount = 1; - return; } else if (icount_align_option) { error_setg(errp, "shift=auto and align=on are incompatible"); + return; } else if (!icount_sleep) { error_setg(errp, "shift=auto and sleep=off are incompatible"); + return; + } + + icount_sleep = sleep; + if (icount_sleep) { + timers_state.icount_warp_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL_RT, + icount_timer_cb, NULL); + } + + icount_align_option = align; + + if (time_shift >= 0) { + timers_state.icount_time_shift = time_shift; + use_icount = 1; + return; } use_icount = 2; -- cgit 1.4.1 From 9ec374a781c34daa6e70fcd885ef30e090cc2384 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 22 Apr 2020 15:07:09 +0200 Subject: cpus: Proper range-checking for -icount shift=N timers_state.icount_time_shift must be in [0,63] to avoid undefined behavior when shifting by it, e.g. in cpu_icount_to_ns(). icount_adjust() clamps it to [0,MAX_ICOUNT_SHIFT], with MAX_ICOUNT_SHIFT = 10. configure_icount() doesn't. Fix that. Fixes: a8bfac37085c3372366d722f131a7e18d664ee4d Cc: Paolo Bonzini Signed-off-by: Markus Armbruster Message-Id: <20200422130719.28225-5-armbru@redhat.com> --- cpus.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'cpus.c') diff --git a/cpus.c b/cpus.c index 1b542b37f9..5670c96bcf 100644 --- a/cpus.c +++ b/cpus.c @@ -25,6 +25,7 @@ #include "qemu/osdep.h" #include "qemu-common.h" #include "qemu/config-file.h" +#include "qemu/cutils.h" #include "migration/vmstate.h" #include "monitor/monitor.h" #include "qapi/error.h" @@ -801,7 +802,6 @@ void configure_icount(QemuOpts *opts, Error **errp) bool sleep = qemu_opt_get_bool(opts, "sleep", true); bool align = qemu_opt_get_bool(opts, "align", false); long time_shift = -1; - char *rem_str = NULL; if (!option && qemu_opt_get(opts, "align")) { error_setg(errp, "Please specify shift option when using align"); @@ -814,9 +814,8 @@ void configure_icount(QemuOpts *opts, Error **errp) } if (strcmp(option, "auto") != 0) { - errno = 0; - time_shift = strtol(option, &rem_str, 0); - if (errno != 0 || *rem_str != '\0' || !strlen(option)) { + if (qemu_strtol(option, NULL, 0, &time_shift) < 0 + || time_shift < 0 || time_shift > MAX_ICOUNT_SHIFT) { error_setg(errp, "icount: Invalid shift value"); return; } -- cgit 1.4.1