Re: [Qemu-devel] [PATCH v4] Do not delete BlockDriverState when deleting the drive

[Markus Armbruster]

[Cc: Justin, because I feel it should go into stable-0.14 as well]

Ryan Harper <address@hidden> writes:

> When removing a drive from the host-side via drive_del we currently have
> the following path:
>
> drive_del
> qemu_aio_flush()
> bdrv_close()    // zaps bs->drv, which makes any subsequent I/O get
>                 // dropped.  Works as designed
> drive_uninit()
> bdrv_delete()   // frees the bs.  Since the device is still connected to
>                 // bs, any subsequent I/O is a use-after-free.
>
> The value of bs->drv becomes unpredictable on free.  As long as it
> remains null, I/O still gets dropped, however it could become non-null
> at any point after the free resulting SEGVs or other QEMU state
> corruption.
>
> To resolve this issue as simply as possible, we can chose to not
> actually delete the BlockDriverState pointer.  Since bdrv_close()
> handles setting the drv pointer to NULL, we just need to remove the
> BlockDriverState from the QLIST that is used to enumerate the block
> devices.  This is currently handled within bdrv_delete, so move this
> into its own function, bdrv_make_anon().
>
> The result is that we can now invoke drive_del, this closes the file
> descriptors and sets BlockDriverState->drv to NULL which prevents futher
> IO to the device, and since we do not free BlockDriverState, we don't
> have to worry about the copy retained in the block devices.
>
> We also don't attempt to remove the qdev property since we are no longer
> deleting the BlockDriverState on drives with associated drives.  This
> also allows for removing Drives with no devices associated either.
>
> Reported-by: Markus Armbruster <address@hidden>
> Signed-off-by: Ryan Harper <address@hidden>

Acked-by: Markus Armbruster <address@hidden>