diff mbox

[2/2] hmp: fix bad value conversion for M type

Message ID 1335483461-31838-3-git-send-email-lcapitulino@redhat.com
State New
Headers show

Commit Message

Luiz Capitulino April 26, 2012, 11:37 p.m. UTC
The M type converts from megabytes to bytes. However, the value can be
negative before the conversion, which will lead to a flawed conversion.

For example, this:

 (qemu) balloon -1000000000000011
 (qemu)

Just "works", but the value passed by the balloon command will be
something else.

This patch fixes this problem by requering a positive value before
converting. There's really no reason to accept a negative value for
the M type.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 monitor.c |    4 ++++
 1 file changed, 4 insertions(+)

Comments

Markus Armbruster April 27, 2012, 8:52 a.m. UTC | #1
Luiz Capitulino <lcapitulino@redhat.com> writes:

> The M type converts from megabytes to bytes. However, the value can be
> negative before the conversion, which will lead to a flawed conversion.
>
> For example, this:
>
>  (qemu) balloon -1000000000000011
>  (qemu)
>
> Just "works", but the value passed by the balloon command will be
> something else.
>
> This patch fixes this problem by requering a positive value before
> converting. There's really no reason to accept a negative value for
> the M type.
>
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> ---
>  monitor.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/monitor.c b/monitor.c
> index 56ee971..58b692b 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3625,6 +3625,10 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon,
>                      monitor_printf(mon, "integer is for 32-bit values\n");
>                      goto fail;
>                  } else if (c == 'M') {
> +                    if (val < 0) {
> +                        monitor_printf(mon, "enter a positive value\n");
> +                        goto fail;
> +                    }
>                      val <<= 20;
>                  }
>                  qdict_put(qdict, key, qint_from_int(val));

Error message doesn't tell the user which argument is wrong.  All the
existing messages are just as bad.  Let's stick to fixing the bug at
hand, and leave decent error messages to another day.

Please fix up the comment on type 'M':

 * 'M'          just like 'l', except in user mode the value is
 *              multiplied by 2^20 (think Mebibyte)

It's no longer "just like 'l'", since unlike 'l' it doesn't accept
negative numbers.
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 56ee971..58b692b 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3625,6 +3625,10 @@  static const mon_cmd_t *monitor_parse_command(Monitor *mon,
                     monitor_printf(mon, "integer is for 32-bit values\n");
                     goto fail;
                 } else if (c == 'M') {
+                    if (val < 0) {
+                        monitor_printf(mon, "enter a positive value\n");
+                        goto fail;
+                    }
                     val <<= 20;
                 }
                 qdict_put(qdict, key, qint_from_int(val));