From: Peter Maydell <peter.maydell@linaro.org>
To: Paul Durrant <paul.durrant@citrix.com>,
QEMU Developers <qemu-devel@nongnu.org>
Cc: Kevin Wolf <kwolf@redhat.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Qemu-block <qemu-block@nongnu.org>, Max Reitz <mreitz@redhat.com>,
Anthony Perard <anthony.perard@citrix.com>,
"open list:X86" <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH] xen-block: stop leaking memory in xen_block_drive_create()
Date: Tue, 19 Feb 2019 16:37:20 +0000 [thread overview]
Message-ID: <CAFEAcA8tv=3isnJLfJx2a4TiEZR9rHzwqj+p=fB_94rJw=k54A__38964.7140794963$1550594305$gmane$org@mail.gmail.com> (raw)
In-Reply-To: <20190219163440.15702-1-paul.durrant@citrix.com>
Hi Paul -- you typoed the qemu-devel list email address; cc'd the right one.
thanks
-- PMM
On Tue, 19 Feb 2019 at 16:35, Paul Durrant <paul.durrant@citrix.com> wrote:
>
> The locally allocated QDict-s need to be freed. ('file_layer' will be
> freed implicitly since it is added as an object to 'driver_layer').
>
> Spotted by Coverity: CID 1398649
>
> While in the neighbourhood free 'driver' and 'filename' as soon as they are
> added to the QDicts. Freeing after the 'done' label doesn't make that much
> sense as, if the error path jumps to that label, the values would be NULL
> anyway.
>
> This patch also makes that more obvious by taking the error path if
> 'params' is NULL and then asserting that both driver and filename are
> non-NULL in the normal path.
>
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Kevin Wolf <kwolf@redhat.com>
> Cc: Max Reitz <mreitz@redhat.com>
> ---
> hw/block/xen-block.c | 16 +++++++++-------
> 1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
> index 37a456c207..70fc2455e8 100644
> --- a/hw/block/xen-block.c
> +++ b/hw/block/xen-block.c
> @@ -743,12 +743,12 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
> }
>
> g_strfreev(v);
> - }
> -
> - if (!filename) {
> - error_setg(errp, "no filename");
> + } else {
> + error_setg(errp, "no params");
> goto done;
> }
> +
> + assert(filename);
> assert(driver);
>
> drive = g_new0(XenBlockDrive, 1);
> @@ -758,6 +758,7 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
>
> qdict_put_str(file_layer, "driver", "file");
> qdict_put_str(file_layer, "filename", filename);
> + g_free(filename);
>
> if (mode && *mode != 'w') {
> qdict_put_bool(file_layer, "read-only", true);
> @@ -793,16 +794,17 @@ static XenBlockDrive *xen_block_drive_create(const char *id,
> driver_layer = qdict_new();
>
> qdict_put_str(driver_layer, "driver", driver);
> + g_free(driver);
> +
> qdict_put_obj(driver_layer, "file", QOBJECT(file_layer));
>
> g_assert(!drive->node_name);
> drive->node_name = xen_block_blockdev_add(drive->id, driver_layer,
> &local_err);
>
> -done:
> - g_free(driver);
> - g_free(filename);
> + qobject_unref(driver_layer);
>
> +done:
> if (local_err) {
> error_propagate(errp, local_err);
> xen_block_drive_destroy(drive, NULL);
> --
> 2.20.1.2.gb21ebb6
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-02-19 16:37 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190219163440.15702-1-paul.durrant@citrix.com>
2019-02-19 16:36 ` [Qemu-devel] [PATCH] xen-block: stop leaking memory in xen_block_drive_create() Paul Durrant
2019-02-25 16:46 ` Anthony PERARD
2019-02-25 16:46 ` Anthony PERARD
2019-02-19 16:36 ` Paul Durrant
2019-02-19 16:37 ` [Qemu-devel] " Peter Maydell
2019-02-19 16:37 ` Peter Maydell [this message]
2019-02-19 16:34 Paul Durrant
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='CAFEAcA8tv=3isnJLfJx2a4TiEZR9rHzwqj+p=fB_94rJw=k54A__38964.7140794963$1550594305$gmane$org@mail.gmail.com' \
--to=peter.maydell@linaro.org \
--cc=anthony.perard@citrix.com \
--cc=kwolf@redhat.com \
--cc=mreitz@redhat.com \
--cc=paul.durrant@citrix.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.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.