Re: [Qemu-devel] [PATCH v3 1/2] block: fix dangling bs->explicit_options in block.c

[Manos Pitsidianakis]

On Fri, Jul 14, 2017 at 09:42:22AM -0500, Eric Blake wrote:

On 07/14/2017 09:35 AM, Manos Pitsidianakis wrote:

In some error paths it is possible to QDECREF a freed dangling
explicit_options, resulting in a heap overflow crash.  For example
bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls
bdrv_close which also unrefs it.

Signed-off-by: Manos Pitsidianakis 
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)


Reviewed-by: Eric Blake 

Can you pinpoint which commit introduced the bug, in order to decide if
this affects 2.9 and should therefore be cc'd to qemu-stable?


I think the particular error path was unreachable in every case since 
bdrv_open_driver() erroneously sets bs->drv to NULL, so bdrv_close never 
performs cleanups. I fix this in the next patch. I am not completely 
sure if it's possible to trigger this otherwise.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v3 1/2] block: fix dangling bs->explicit_options in block.c

[Eric Blake]

On 07/14/2017 09:35 AM, Manos Pitsidianakis wrote:
> In some error paths it is possible to QDECREF a freed dangling
> explicit_options, resulting in a heap overflow crash.  For example
> bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls
> bdrv_close which also unrefs it.
> 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  block.c | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Eric Blake 

Can you pinpoint which commit introduced the bug, in order to decide if
this affects 2.9 and should therefore be cc'd to qemu-stable?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 1/2] block: fix dangling bs->explicit_options in block.c

[Manos Pitsidianakis]

In some error paths it is possible to QDECREF a freed dangling
explicit_options, resulting in a heap overflow crash.  For example
bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls
bdrv_close which also unrefs it.

Signed-off-by: Manos Pitsidianakis 
---
 block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/block.c b/block.c
index 98a9209371..96b912d229 100644
--- a/block.c
+++ b/block.c
@@ -2608,6 +2608,7 @@ fail:
 QDECREF(bs->options);
 QDECREF(options);
 bs->options = NULL;
+bs->explicit_options = NULL;
 bdrv_unref(bs);
 error_propagate(errp, local_err);
 return NULL;
@@ -3087,6 +3088,7 @@ static void bdrv_close(BlockDriverState *bs)
 QDECREF(bs->options);
 QDECREF(bs->explicit_options);
 bs->options = NULL;
+bs->explicit_options = NULL;
 QDECREF(bs->full_open_options);
 bs->full_open_options = NULL;
 }
-- 
2.11.0