From: Michael Roth <mdroth@linux.vnet.ibm.com>
To: qemu-devel@nongnu.org
Cc: famz@redhat.com, peter.maydell@linaro.org, armbru@redhat.com,
lcapitulino@redhat.com
Subject: [Qemu-devel] [PATCH] qapi: zero-initialize all QMP command parameters
Date: Tue, 20 May 2014 12:20:39 -0500 [thread overview]
Message-ID: <1400606439-19899-1-git-send-email-mdroth@linux.vnet.ibm.com> (raw)
In general QMP command parameter values are specified by consumers of the
QMP/HMP interface, but in the case of optional parameters these values may
be left uninitialized.
It is considered a bug for code to make use of optional parameters that have
not been flagged as being present by the marshalling code (via corresponding
has_<parameter> parameter), however our marshalling code will still pass
these uninitialized values on to the corresponding QMP function (to then
be ignored). Some compilers (clang in particular) consider this unsafe
however, and generate warnings as a result. As reported by Peter Maydell:
This is something clang's -fsanitize=undefined spotted. The
code generated by qapi-commands.py in qmp-marshal.c for
qmp_marshal_* functions where there are some optional
arguments looks like this:
bool has_force = false;
bool force;
mi = qmp_input_visitor_new_strict(QOBJECT(args));
v = qmp_input_get_visitor(mi);
visit_type_str(v, &device, "device", errp);
visit_start_optional(v, &has_force, "force", errp);
if (has_force) {
visit_type_bool(v, &force, "force", errp);
}
visit_end_optional(v, errp);
qmp_input_visitor_cleanup(mi);
if (error_is_set(errp)) {
goto out;
}
qmp_eject(device, has_force, force, errp);
In the case where has_force is false, we never initialize
force, but then we use it by passing it to qmp_eject.
I imagine we don't then actually use the value, but clang
complains in particular for 'bool' variables because the value
that ends up being loaded from memory for 'force' is not either
0 or 1 (being uninitialized stack contents).
Fix this by initializing all QMP command parameters to {0} in the
marshalling code prior to passing them on to the QMP functions.
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Tested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
scripts/qapi-commands.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py
index 386f17e..7d93d01 100644
--- a/scripts/qapi-commands.py
+++ b/scripts/qapi-commands.py
@@ -111,7 +111,7 @@ bool has_%(argname)s = false;
argname=c_var(argname), argtype=c_type(argtype))
else:
ret += mcgen('''
-%(argtype)s %(argname)s;
+%(argtype)s %(argname)s = {0};
''',
argname=c_var(argname), argtype=c_type(argtype))
--
1.9.1
next reply other threads:[~2014-05-20 17:21 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-20 17:20 Michael Roth [this message]
2014-05-20 17:59 ` [Qemu-devel] [PATCH] qapi: zero-initialize all QMP command parameters Markus Armbruster
2014-05-20 18:21 ` Peter Maydell
2014-05-20 18:41 ` Michael Roth
2014-05-21 13:27 ` Luiz Capitulino
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=1400606439-19899-1-git-send-email-mdroth@linux.vnet.ibm.com \
--to=mdroth@linux.vnet.ibm.com \
--cc=armbru@redhat.com \
--cc=famz@redhat.com \
--cc=lcapitulino@redhat.com \
--cc=peter.maydell@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).