From 79c7744efb390c11a5ca0f70e5170c40f9b3b2f1 Mon Sep 17 00:00:00 2001
From: Jin Yu <jin.yu@intel.com>
Date: Wed, 29 Jul 2020 21:24:31 +0800
Subject: [PATCH] virtio: fix virtio hw double free issue
During virtio_pci_dev_probe, if enum_cb fails, hw needs
to be released. But in bdev_virtio, if vdev fails after
initialization, it will enter the bdev destruction process
which call the modern_destruct_dev function and hw will
be released during the process. So we will encounter the
problem of hw being released twice.
Change-Id: Ifba35284c072355ba0e10428b597a1894d32d59e
Signed-off-by: Jin Yu <jin.yu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/3564
Community-CI: Mellanox Build Bot
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Shuhei Matsumoto <shuhei.matsumoto.xt@hitachi.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
---
lib/virtio/virtio_pci.c | 7 ++++---
module/bdev/virtio/bdev_virtio_blk.c | 18 +++++++++---------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/lib/virtio/virtio_pci.c b/lib/virtio/virtio_pci.c
index 646f77c1aa..1954043da1 100644
--- a/lib/virtio/virtio_pci.c
+++ b/lib/virtio/virtio_pci.c
@@ -224,10 +224,11 @@ static void
modern_destruct_dev(struct virtio_dev *vdev)
{
struct virtio_hw *hw = vdev->ctx;
- struct spdk_pci_device *pci_dev = hw->pci_dev;
- free_virtio_hw(hw);
- spdk_pci_device_detach(pci_dev);
+ if (hw != NULL) {
+ free_virtio_hw(hw);
+ spdk_pci_device_detach(hw->pci_dev);
+ }
}
static uint8_t
diff --git a/module/bdev/virtio/bdev_virtio_blk.c b/module/bdev/virtio/bdev_virtio_blk.c
index 99653e238b..8730f39db7 100644
--- a/module/bdev/virtio/bdev_virtio_blk.c
+++ b/module/bdev/virtio/bdev_virtio_blk.c
@@ -554,9 +554,7 @@ virtio_pci_blk_dev_create(const char *name, struct virtio_pci_ctx *pci_ctx)
rc = virtio_dev_reset(vdev, VIRTIO_BLK_DEV_SUPPORTED_FEATURES);
if (rc != 0) {
- virtio_dev_destruct(vdev);
- free(bvdev);
- return NULL;
+ goto fail;
}
/* TODO: add a way to limit usable virtqueues */
@@ -565,9 +563,7 @@ virtio_pci_blk_dev_create(const char *name, struct virtio_pci_ctx *pci_ctx)
&num_queues, sizeof(num_queues));
if (rc) {
SPDK_ERRLOG("%s: config read failed: %s\n", vdev->name, spdk_strerror(-rc));
- virtio_dev_destruct(vdev);
- free(bvdev);
- return NULL;
+ goto fail;
}
} else {
num_queues = 1;
@@ -575,12 +571,16 @@ virtio_pci_blk_dev_create(const char *name, struct virtio_pci_ctx *pci_ctx)
rc = virtio_blk_dev_init(bvdev, num_queues);
if (rc != 0) {
- virtio_dev_destruct(vdev);
- free(bvdev);
- return NULL;
+ goto fail;
}
return bvdev;
+
+fail:
+ vdev->ctx = NULL;
+ virtio_dev_destruct(vdev);
+ free(bvdev);
+ return NULL;
}
static struct virtio_blk_dev *