Hi
On Tue, Sep 15, 2026 at 11:54 AM Bin Guo <[email protected]> wrote:
>
> The HMP expression evaluator performed *, +, - and unary - on int64_t
> values without checking for overflow. A malformed expression such as
> "0x7fffffffffffffff + 1" would silently wrap around to INT64_MIN and
> could then be passed as a physical address to the "xp" command.
>
> Use the checked-arithmetic helpers from host-utils.h:
> - smul64_overflow() for "*"
> - sadd64_overflow()/ssub64_overflow() for "+"/"-"
> - ssub64_overflow(0, n) for unary "-"
> - an explicit check for INT64_MIN / -1 and INT64_MIN % -1
>
> Also check the "M" (MiB) suffix multiplier used by commands such as
> "balloon".
>
> Overflow now raises an "integer overflow" error via expr_error() or
> monitor_hmp_printf()/goto fail instead of propagating a wrapped value.
>
> Add a qtest that verifies the evaluator reports "integer overflow" for
> the classic overflow cases and still accepts non-overflowing values.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/4463
> Signed-off-by: Bin Guo <[email protected]>
> ---
> monitor/hmp.c | 26 +++++++--
> tests/qtest/hmp-expr-overflow-test.c | 81 ++++++++++++++++++++++++++++
> tests/qtest/meson.build | 1 +
> 3 files changed, 103 insertions(+), 5 deletions(-)
> create mode 100644 tests/qtest/hmp-expr-overflow-test.c
>
> diff --git a/monitor/hmp.c b/monitor/hmp.c
> index 488ec23937..e6be921770 100644
> --- a/monitor/hmp.c
> +++ b/monitor/hmp.c
> @@ -35,6 +35,7 @@
> #include "qemu/config-file.h"
> #include "qemu/ctype.h"
> #include "qemu/cutils.h"
> +#include "qemu/host-utils.h"
> #include "qemu/log.h"
> #include "qemu/option.h"
> #include "qemu/base-arch-defs.h"
> @@ -480,7 +481,10 @@ static int64_t expr_unary(MonitorHMP *mon)
> break;
> case '-':
> next();
> - n = -expr_unary(mon);
> + n = expr_unary(mon);
> + if (ssub64_overflow(0, n, &n)) {
> + expr_error(mon, "integer overflow");
> + }
> break;
> case '~':
> next();
> @@ -571,13 +575,18 @@ static int64_t expr_prod(MonitorHMP *mon)
> switch (op) {
> default:
> case '*':
> - val *= val2;
> + if (smul64_overflow(val, val2, &val)) {
> + expr_error(mon, "integer overflow");
> + }
> break;
> case '/':
> case '%':
> if (val2 == 0) {
> expr_error(mon, "division by zero");
> }
> + if (val == INT64_MIN && val2 == -1) {
> + expr_error(mon, "integer overflow");
> + }
> if (op == '/') {
> val /= val2;
> } else {
> @@ -632,9 +641,13 @@ static int64_t expr_sum(MonitorHMP *mon)
> next();
> val2 = expr_logic(mon);
> if (op == '+') {
> - val += val2;
> + if (sadd64_overflow(val, val2, &val)) {
> + expr_error(mon, "integer overflow");
> + }
> } else {
> - val -= val2;
> + if (ssub64_overflow(val, val2, &val)) {
> + expr_error(mon, "integer overflow");
> + }
> }
> }
> return val;
> @@ -1028,7 +1041,10 @@ static QDict *monitor_parse_arguments(MonitorHMP *mon,
> monitor_hmp_printf(mon, "enter a positive value\n");
> goto fail;
> }
> - val *= MiB;
> + if (smul64_overflow(val, MiB, &val)) {
> + monitor_hmp_printf(mon, "integer overflow\n");
> + goto fail;
> + }
> }
> qdict_put_int(qdict, key, val);
> }
> diff --git a/tests/qtest/hmp-expr-overflow-test.c
> b/tests/qtest/hmp-expr-overflow-test.c
> new file mode 100644
> index 0000000000..29ef880993
> --- /dev/null
> +++ b/tests/qtest/hmp-expr-overflow-test.c
> @@ -0,0 +1,81 @@
> +/*
> + * QTest regression test for HMP expression evaluator overflow.
> + *
> + * Copyright (c) 2026 Bin Guo
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +
> +static void assert_overflow_response(QTestState *qts, const char *cmd)
> +{
> + g_autofree char *resp = qtest_hmp(qts, "%s", cmd);
> +
> + g_assert(strstr(resp, "integer overflow") != NULL);
> +}
> +
> +static void assert_ok_response(QTestState *qts, const char *cmd)
> +{
> + g_autofree char *resp = qtest_hmp(qts, "%s", cmd);
> +
> + g_assert(strstr(resp, "integer overflow") == NULL);
> + g_assert(strstr(resp, "error") == NULL);
> +}
> +
> +static void test_expr_overflow(void)
> +{
> + QTestState *qts = qtest_init("-M none -m 2");
> +
> + /* Addition overflow */
> + assert_overflow_response(qts, "print 0x7fffffffffffffff + 1");
> +
> + /* Subtraction overflow */
> + assert_overflow_response(qts, "print -0x8000000000000000 - 1");
That's actually testing unary overflow of -0x8000000000000000
Bypass it with ~0x7fffffffffffffff (INT64_MIN) ?
> +
> + /* Multiplication overflow */
> + assert_overflow_response(qts, "print 0x4000000000000000 * 2");
> +
> + /* Unary negation of INT64_MIN */
> + assert_overflow_response(qts, "print -0x8000000000000000");
> +
> + /* Division overflow */
> + assert_overflow_response(qts, "print -0x8000000000000000 / -1");
> + assert_overflow_response(qts, "print -0x8000000000000000 % -1");
> +
> + /* Sanity: non-overflowing expressions still work */
> + assert_ok_response(qts, "print 1 + 1");
> + assert_ok_response(qts, "print 0x7fffffffffffffff");
> + assert_ok_response(qts, "print -0x7fffffffffffffff");
> +
> + qtest_quit(qts);
> +}
> +
> +static void test_balloon_m_overflow(void)
> +{
> + QTestState *qts;
> +
> + /* q35 is x86-only; skip this test on other architectures. */
> + if (!qtest_has_machine("q35")) {
g_test_skip("Machine 'q35' is not available");
> + return;
> + }
> +
> + /* Balloon command takes an 'M' suffix size argument in MB. */
> + qts = qtest_init("-M q35 -m 128 -device virtio-balloon-pci");
> +
> + /* 0x7fffffffffffffff is positive as int64_t, but * MiB overflows. */
> + assert_overflow_response(qts, "balloon 0x7fffffffffffffff");
> +
> + qtest_quit(qts);
> +}
> +
> +int main(int argc, char **argv)
> +{
> + g_test_init(&argc, &argv, NULL);
> +
> + qtest_add_func("hmp/expr-overflow", test_expr_overflow);
> + qtest_add_func("hmp/balloon-m-overflow", test_balloon_m_overflow);
> +
> + return g_test_run();
> +}
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index c3593f7530..89172ac45b 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -30,6 +30,7 @@ qtests_generic = [
> if have_hmp
> qtests_generic += [
> 'test-hmp',
> + 'hmp-expr-overflow-test',
> ]
> endif
> qtests_generic += [
> --
> 2.50.1 (Apple Git-155)
>
>
otherwise, lgtm
--
Marc-André Lureau
