qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: qemu-devel@nongnu.org, kraxel@redhat.com, berrange@redhat.com,
	philmd@linaro.org
Subject: Re: [PATCH v3 02/18] ui: Fix silent truncation of numeric keys in HMP sendkey
Date: Wed, 4 Jan 2023 16:19:03 +0000	[thread overview]
Message-ID: <Y7Wm94X4gxl738Fd@work-vm> (raw)
In-Reply-To: <20221220090645.2844881-3-armbru@redhat.com>

* Markus Armbruster (armbru@redhat.com) wrote:
> Keys are int.  HMP sendkey assigns them from the value strtoul(),
> silently truncating values greater than INT_MAX.  Fix to reject them.
> 
> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
> won't complain.

Last time through you said you could switch to qemu_strtoui, but
I just noticed we've actually got a qemu_strto*i* - that
would remove the value comparison

Dave

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  monitor/hmp-cmds.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ed78a87ddd..b8e294e6fa 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1549,8 +1549,13 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>          v = g_malloc0(sizeof(*v));
>  
>          if (strstart(keys, "0x", NULL)) {
> -            char *endp;
> -            int value = strtoul(keys, &endp, 0);
> +            const char *endp;
> +            unsigned long value;
> +
> +            if (qemu_strtoul(keys, &endp, 0, &value) < 0
> +                || value >= INT_MAX) {
> +                goto err_out;
> +            }
>              assert(endp <= keys + keyname_len);
>              if (endp != keys + keyname_len) {
>                  goto err_out;
> -- 
> 2.38.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK



  reply	other threads:[~2023-01-04 16:19 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-20  9:06 [PATCH v3 00/18] ui: Move and clean up monitor command code Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 01/18] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
2022-12-20  9:10   ` Daniel P. Berrangé
2022-12-20  9:06 ` [PATCH v3 02/18] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
2023-01-04 16:19   ` Dr. David Alan Gilbert [this message]
2023-01-09 11:50     ` Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 03/18] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 04/18] Revert "hmp: info spice: take out webdav" Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 05/18] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 06/18] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 07/18] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 08/18] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
2022-12-20 11:13   ` Philippe Mathieu-Daudé
2022-12-20  9:06 ` [PATCH v3 09/18] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 10/18] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 11/18] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/ Markus Armbruster
2022-12-20  9:11   ` Daniel P. Berrangé
2022-12-20 11:22   ` Philippe Mathieu-Daudé
2022-12-20 11:49     ` Markus Armbruster
2022-12-20 12:29       ` Philippe Mathieu-Daudé
2022-12-20 15:30         ` Markus Armbruster
2022-12-20 16:35           ` Philippe Mathieu-Daudé
2022-12-20  9:06 ` [PATCH v3 13/18] ui: Improve "change vnc" error reporting Markus Armbruster
2022-12-20 11:23   ` Philippe Mathieu-Daudé
2022-12-20  9:06 ` [PATCH v3 14/18] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 15/18] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/ Markus Armbruster
2022-12-20  9:15   ` Daniel P. Berrangé
2022-12-20 11:20     ` Markus Armbruster
2022-12-20 11:17   ` Philippe Mathieu-Daudé
2023-01-09 14:35     ` Markus Armbruster
2022-12-20  9:06 ` [PATCH v3 17/18] ui: Don't check for mode change after mouse_set error Markus Armbruster
2022-12-20  9:16   ` Daniel P. Berrangé
2022-12-20  9:06 ` [PATCH v3 18/18] ui: Simplify control flow in qemu_mouse_set() Markus Armbruster
2022-12-20  9:17   ` Daniel P. Berrangé
2022-12-20 10:52 ` [PATCH v3 00/18] ui: Move and clean up monitor command code Philippe Mathieu-Daudé
2022-12-20 11:17   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y7Wm94X4gxl738Fd@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).