[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] virtio-blk: check for NULL BlockDriverState
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-devel] [PATCH] virtio-blk: check for NULL BlockDriverState |
Date: |
Wed, 24 Jan 2018 11:31:23 +0000 |
User-agent: |
Mutt/1.9.1 (2017-09-22) |
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 <address@hidden>
> Reviewed-by: Karl Heubaum <address@hidden>
> Reviewed-by: Ameya More <address@hidden>
> ---
> 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