From: Markus Armbruster <armbru@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-devel@nongnu.org
Subject: Re: [PATCH] vl: fix leak of qdict_crumple return value
Date: Thu, 08 Jul 2021 11:05:13 +0200 [thread overview]
Message-ID: <87k0m19nw6.fsf@dusky.pond.sub.org> (raw)
In-Reply-To: <20210707121545.361829-1-pbonzini@redhat.com> (Paolo Bonzini's message of "Wed, 7 Jul 2021 14:15:44 +0200")
Paolo Bonzini <pbonzini@redhat.com> writes:
> Coverity reports that qemu_parse_config_group is returning without
> unrefing the "crumpled" dictionary in case its top level item is a
> list. But actually the contract with qemu_record_config_group is
> the same as for qemu_parse_config_group itself: if those function
> need to stash the dictionary they get, they have to take a reference
> themselves (currently this is never the case for either function).
> Therefore, just add an unconditional qobject_unref(crumpled) to
> qemu_parse_config_group.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Do we want
Fixes: c0d4aa82f895af67cbf7772324e05605e22b4162
here?
> ---
> softmmu/vl.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 2004d57108..7b54ddf6f4 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2196,9 +2196,10 @@ static void qemu_parse_config_group(const char *group, QDict *qdict,
> if (qobject_type(crumpled) != QTYPE_QDICT) {
> assert(qobject_type(crumpled) == QTYPE_QLIST);
> error_setg(errp, "Lists cannot be at top level of a configuration section");
> - return;
> + } else {
> + qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
> }
> - qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
> + qobject_unref(crumpled);
> }
>
> static void qemu_read_default_config_file(Error **errp)
Minimally invasive fix, but the result is a bit awkward. Possibly
neater:
if (qobject_type(crumpled) == QTYPE_QLIST) {
error_setg(errp,
"Lists cannot be at top level of a configuration section");
} else {
assert(qobject_type(crumpled) == QTYPE_QDICT);
qemu_record_config_group(group, qobject_to(QDict, crumpled),
false, errp);
}
qobject_unref(crumpled);
Regardless:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
next prev parent reply other threads:[~2021-07-08 9:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-07 12:15 [PATCH] vl: fix leak of qdict_crumple return value Paolo Bonzini
2021-07-08 9:05 ` Markus Armbruster [this message]
2021-07-08 9:41 ` Paolo Bonzini
2021-07-08 9:42 ` Paolo Bonzini
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=87k0m19nw6.fsf@dusky.pond.sub.org \
--to=armbru@redhat.com \
--cc=pbonzini@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.