Amit Shah <amit.s...@redhat.com> writes:
> Negative balloon values don't make sense, ignore them.
>
> Reported-by: Mike Cao <b...@redhat.com>
> Signed-off-by: Amit Shah <amit.s...@redhat.com>
> ---
> I'm not sure if error_report is the right thing to use or should a new
> qerror_report() be used. Luiz, comments?
Since do_balloon() has been converted to qerror already, you should use
qerror_report(). Something like this should do[*]:
qerror_report(QERR_INVALID_PARAMETER_VALUE, "target", "a size")
> balloon.c | 8 +++++++-
> 1 files changed, 7 insertions(+), 1 deletions(-)
>
> diff --git a/balloon.c b/balloon.c
> index cf9e3b2..e0ff97f 100644
> --- a/balloon.c
> +++ b/balloon.c
> @@ -51,12 +51,16 @@ int qemu_add_balloon_handler(QEMUBalloonEvent *event_func,
> return 0;
> }
>
> -static int qemu_balloon(ram_addr_t target)
> +static int qemu_balloon(long long target)
> {
> if (!balloon_event_fn) {
> return 0;
> }
> trace_balloon_event(balloon_opaque, target);
> + if (target < 0) {
> + error_report("Ignoring negative balloon value");
> + return -1;
> + }
> balloon_event_fn(balloon_opaque, target);
> return 1;
> }
Monitor argument type is 'M', i.e. target_long. Caller do_balloon() it
as int64_t. Argument passing casts it to ram_addr_t, which is unsigned.
Negative arguments get misinterpreted.
You fix it by converting to long long instead, then rejecting negative
arguments.
I think do_balloon() is a more natural place to check the argument
range. Permits keeping qemu_balloon()'s parameter type as is.
> @@ -150,6 +154,8 @@ int do_balloon(Monitor *mon, const QDict *params,
> if (ret == 0) {
> qerror_report(QERR_DEVICE_NOT_ACTIVE, "balloon");
> return -1;
> + } else if (ret < 0) {
> + return -1;
> }
>
> cb(opaque, NULL);
[*] Yes, that results in a sub-par error message for humans. Human
users are advised to appreciate that the error message was created with
proper object-oriented techniques.
