On Mon, Jan 22, 2018 at 09:01:49AM -0600, Mark Kanda wrote:
> Add a BlockDriverState NULL check to virtio_blk_handle_request()
> to prevent a segfault if the drive is forcibly removed using HMP
> 'drive_del' (without performing a hotplug 'device_del' first).
>
> Signed-off-by: Mark Kanda <mark.ka...@oracle.com>
> Reviewed-by: Karl Heubaum <karl.heub...@oracle.com>
> Reviewed-by: Ameya More <ameya.m...@oracle.com>
> ---
> hw/block/virtio-blk.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> index b1532e4..76ddbbf 100644
> --- a/hw/block/virtio-blk.c
> +++ b/hw/block/virtio-blk.c
> @@ -507,6 +507,13 @@ static int virtio_blk_handle_request(VirtIOBlockReq
> *req, MultiReqBuffer *mrb)
> return -1;
> }
>
> + /* If the drive was forcibly removed (e.g. HMP 'drive_del'), the block
> + * driver state may be NULL and there is nothing left to do. */
> + if (!blk_bs(req->dev->blk)) {Adding Markus Armbruster to check my understanding of drive_del: 1. If id is a node name (e.g. created via blockdev-add) then attempting to remove the root node produces the "Node %s is in use" error. In that case this patch isn't needed. 2. If id is a BlockBackend (e.g. created via -drive) then removing the root node is allowed. The BlockBackend stays in place but blk->root becomes NULL, hence this patch is needed. Markus: What are the valid use cases for #2? If blk->bs becomes NULL I would think a lot more code beyond virtio-blk can segfault. > + virtio_error(vdev, "virtio-blk BlockDriverState is NULL"); > + return -1; > + } > + > /* We always touch the last byte, so just see how big in_iov is. */ > req->in_len = iov_size(in_iov, in_num); > req->in = (void *)in_iov[in_num - 1].iov_base > -- > 1.8.3.1 >
signature.asc
Description: PGP signature
