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"); + + /* 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")) { + 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)
