[Qemu-devel] [PATCH] qemu-img: Fix options leakage in img_rebase()

Max Reitz posted 1 patch 4 years, 11 months ago
Test s390x passed
Test checkpatch passed
Test asan passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test FreeBSD passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190528195338.12376-1-mreitz@redhat.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
qemu-img.c | 3 +++
1 file changed, 3 insertions(+)
[Qemu-devel] [PATCH] qemu-img: Fix options leakage in img_rebase()
Posted by Max Reitz 4 years, 11 months ago
img_rebase() can leak a QDict in two occasions.  Fix it.

Coverity: CID 1401416
Fixes: d16699b64671466b42079c45b89127aeea1ca565
Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qemu-img.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/qemu-img.c b/qemu-img.c
index b0535919b1..86e1923acf 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3347,6 +3347,7 @@ static int img_rebase(int argc, char **argv)
                                                              out_baseimg,
                                                              &local_err);
             if (local_err) {
+                qobject_unref(options);
                 error_reportf_err(local_err,
                                   "Could not resolve backing filename: ");
                 ret = -1;
@@ -3359,7 +3360,9 @@ static int img_rebase(int argc, char **argv)
              */
             prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
             if (prefix_chain_bs) {
+                qobject_unref(options);
                 g_free(out_real_path);
+
                 blk_new_backing = blk_new(BLK_PERM_CONSISTENT_READ,
                                           BLK_PERM_ALL);
                 ret = blk_insert_bs(blk_new_backing, prefix_chain_bs,
-- 
2.21.0


Re: [Qemu-devel] [PATCH] qemu-img: Fix options leakage in img_rebase()
Posted by Kevin Wolf 4 years, 10 months ago
Am 28.05.2019 um 21:53 hat Max Reitz geschrieben:
> img_rebase() can leak a QDict in two occasions.  Fix it.
> 
> Coverity: CID 1401416
> Fixes: d16699b64671466b42079c45b89127aeea1ca565
> Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Is this based on some other patch? The second hunk doesn't apply.

Kevin

Re: [Qemu-devel] [PATCH] qemu-img: Fix options leakage in img_rebase()
Posted by Max Reitz 4 years, 10 months ago
On 30.05.19 11:40, Kevin Wolf wrote:
> Am 28.05.2019 um 21:53 hat Max Reitz geschrieben:
>> img_rebase() can leak a QDict in two occasions.  Fix it.
>>
>> Coverity: CID 1401416
>> Fixes: d16699b64671466b42079c45b89127aeea1ca565
>> Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
> 
> Is this based on some other patch? The second hunk doesn't apply.

Yes, it’s based on 330c72957196e0ae382abcaa97ebf4eb9bc8574f, as the
commit message says.  At the time I sent this patch, that was just in my
pull request, now it’s in master.

Max

Re: [Qemu-devel] [Qemu-block] [PATCH] qemu-img: Fix options leakage in img_rebase()
Posted by John Snow 4 years, 10 months ago

On 5/28/19 3:53 PM, Max Reitz wrote:
> img_rebase() can leak a QDict in two occasions.  Fix it.
> 
> Coverity: CID 1401416
> Fixes: d16699b64671466b42079c45b89127aeea1ca565
> Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index b0535919b1..86e1923acf 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3347,6 +3347,7 @@ static int img_rebase(int argc, char **argv)
>                                                               out_baseimg,
>                                                               &local_err);
>              if (local_err) {
> +                qobject_unref(options);
>                  error_reportf_err(local_err,
>                                    "Could not resolve backing filename: ");
>                  ret = -1;
> @@ -3359,7 +3360,9 @@ static int img_rebase(int argc, char **argv)
>               */
>              prefix_chain_bs = bdrv_find_backing_image(bs, out_real_path);
>              if (prefix_chain_bs) {
> +                qobject_unref(options);
>                  g_free(out_real_path);
> +
>                  blk_new_backing = blk_new(BLK_PERM_CONSISTENT_READ,
>                                            BLK_PERM_ALL);
>                  ret = blk_insert_bs(blk_new_backing, prefix_chain_bs,
> 

I was going to ask about the other branch after this one, but:
"The reference to the QDict belongs to the block layer after the call
(even on failure)"

"Oh, well... OK."

Reviewed-by: John Snow <jsnow@redhat.com>

Re: [Qemu-devel] [PATCH] qemu-img: Fix options leakage in img_rebase()
Posted by Max Reitz 4 years, 10 months ago
On 28.05.19 21:53, Max Reitz wrote:
> img_rebase() can leak a QDict in two occasions.  Fix it.
> 
> Coverity: CID 1401416
> Fixes: d16699b64671466b42079c45b89127aeea1ca565
> Fixes: 330c72957196e0ae382abcaa97ebf4eb9bc8574f
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  qemu-img.c | 3 +++
>  1 file changed, 3 insertions(+)

Applied to my block branch.

Max