| Message ID | 20170714143548.32559-2-el13635@mail.ntua.gr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <el13635@mail.ntua.gr> > --- > block.c | 2 ++ > 1 file changed, 2 insertions(+) Reviewed-by: Eric Blake <eblake@redhat.com> 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?
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 <el13635@mail.ntua.gr> >> --- >> block.c | 2 ++ >> 1 file changed, 2 insertions(+) > >Reviewed-by: Eric Blake <eblake@redhat.com> > >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.
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; }
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 <el13635@mail.ntua.gr> --- block.c | 2 ++ 1 file changed, 2 insertions(+)