[Qemu-devel] [PATCH for-2.1] block/cow: Avoid use of uninitialized cow_bs in error path

[Qemu-devel] [PATCH for-2.1] block/cow: Avoid use of uninitialized cow_bs in error path

[Peter Maydell]

Commit 25814e8987 introduced an error-exit code path which does
a "goto exit" before the cow_bs variable is initialized, meaning
we would call bdrv_unref() on an uninitialized variable and
likely segfault. Fix this by moving the NULL-initialization
to the top of the function and making the exit code path handle
the case where it is NULL.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 block/cow.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/cow.c b/block/cow.c
index 8f81ee6..6ee4833 100644
--- a/block/cow.c
+++ b/block/cow.c
@@ -332,7 +332,7 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
     char *image_filename = NULL;
     Error *local_err = NULL;
     int ret;
-    BlockDriverState *cow_bs;
+    BlockDriverState *cow_bs = NULL;
 
     /* Read out options */
     image_sectors = qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0) / 512;
@@ -344,7 +344,6 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
         goto exit;
     }
 
-    cow_bs = NULL;
     ret = bdrv_open(&cow_bs, filename, NULL, NULL,
                     BDRV_O_RDWR | BDRV_O_PROTOCOL, NULL, &local_err);
     if (ret < 0) {
@@ -383,7 +382,9 @@ static int cow_create(const char *filename, QemuOpts *opts, Error **errp)
 
 exit:
     g_free(image_filename);
-    bdrv_unref(cow_bs);
+    if (cow_bs) {
+        bdrv_unref(cow_bs);
+    }
     return ret;
 }
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH for-2.1] block/cow: Avoid use of uninitialized cow_bs in error path

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 786 bytes --]

On 06/30/2014 12:03 PM, Peter Maydell wrote:
> Commit 25814e8987 introduced an error-exit code path which does
> a "goto exit" before the cow_bs variable is initialized, meaning
> we would call bdrv_unref() on an uninitialized variable and
> likely segfault. Fix this by moving the NULL-initialization
> to the top of the function and making the exit code path handle
> the case where it is NULL.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  block/cow.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

(and I obviously missed catching this on my first review of the patch)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.1] block/cow: Avoid use of uninitialized cow_bs in error path

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 667 bytes --]

On Mon, Jun 30, 2014 at 07:03:37PM +0100, Peter Maydell wrote:
> Commit 25814e8987 introduced an error-exit code path which does
> a "goto exit" before the cow_bs variable is initialized, meaning
> we would call bdrv_unref() on an uninitialized variable and
> likely segfault. Fix this by moving the NULL-initialization
> to the top of the function and making the exit code path handle
> the case where it is NULL.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  block/cow.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]