From c47cc3abda138e4a250ccc0e1b93766eee944146 Mon Sep 17 00:00:00 2001 From: Jakub Michalski Date: Mon, 9 Jun 2025 13:10:32 +0200 Subject: [PATCH] drivers: virtio: fix cleanup upon failure during virtqueues initialization This commit fixes infinite loop reported in https://github.com/zephyrproject-rtos/zephyr/issues/91242, not deallocating array with virtqueues and not deactivating the virtqueues in case of failure Signed-off-by: Jakub Michalski --- drivers/virtio/virtio_mmio.c | 33 +++++++++++++++++++++++---------- drivers/virtio/virtio_pci.c | 34 ++++++++++++++++++++++++---------- 2 files changed, 47 insertions(+), 20 deletions(-) diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c index 814278333aa..3717f86240d 100644 --- a/drivers/virtio/virtio_mmio.c +++ b/drivers/virtio/virtio_mmio.c @@ -246,30 +246,43 @@ static int virtio_mmio_set_virtqueues(const struct device *dev, uint16_t queue_c } data->virtqueue_count = queue_count; + int ret = 0; + int created_queues = 0; + int activated_queues = 0; + for (int i = 0; i < queue_count; i++) { virtio_mmio_write32(dev, VIRTIO_MMIO_QUEUE_SEL, i); const uint16_t queue_size = cb(i, virtio_mmio_read32(dev, VIRTIO_MMIO_QUEUE_SIZE_MAX), opaque); - int ret = virtq_create(&data->virtqueues[i], queue_size); - + ret = virtq_create(&data->virtqueues[i], queue_size); if (ret != 0) { - for (int j = 0; j < i; i++) { - virtq_free(&data->virtqueues[j]); - } - return ret; + goto fail; } + created_queues++; + ret = virtio_mmio_set_virtqueue(dev, i, &data->virtqueues[i]); if (ret != 0) { - for (int j = 0; j < i; i++) { - virtq_free(&data->virtqueues[j]); - } - return ret; + goto fail; } + activated_queues++; } return 0; + +fail: + for (int j = 0; j < activated_queues; j++) { + virtio_mmio_write32(dev, VIRTIO_MMIO_QUEUE_SEL, j); + virtio_mmio_write32(dev, VIRTIO_MMIO_QUEUE_READY, 0); + } + for (int j = 0; j < created_queues; j++) { + virtq_free(&data->virtqueues[j]); + } + k_free(data->virtqueues); + data->virtqueue_count = 0; + + return ret; } static int virtio_mmio_init_virtqueues(const struct device *dev, uint16_t num_queues, diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c index 8ec095691c9..99f86ff32e3 100644 --- a/drivers/virtio/virtio_pci.c +++ b/drivers/virtio/virtio_pci.c @@ -271,30 +271,44 @@ static int virtio_pci_init_virtqueues( } data->virtqueue_count = queue_count; + int ret = 0; + int created_queues = 0; + int activated_queues = 0; + for (int i = 0; i < queue_count; i++) { data->common_cfg->queue_select = sys_cpu_to_le16(i); barrier_dmem_fence_full(); uint16_t queue_size = cb(i, sys_le16_to_cpu(data->common_cfg->queue_size), opaque); - int ret = virtq_create(&data->virtqueues[i], queue_size); - + ret = virtq_create(&data->virtqueues[i], queue_size); if (ret != 0) { - for (int j = 0; j < i; i++) { - virtq_free(&data->virtqueues[j]); - } - return ret; + goto fail; } + created_queues++; + ret = virtio_pci_set_virtqueue(dev, i, &data->virtqueues[i]); if (ret != 0) { - for (int j = 0; j < i; i++) { - virtq_free(&data->virtqueues[j]); - } - return ret; + goto fail; } + activated_queues++; } return 0; + +fail: + for (int j = 0; j < activated_queues; j++) { + data->common_cfg->queue_select = sys_cpu_to_le16(j); + barrier_dmem_fence_full(); + data->common_cfg->queue_enable = sys_cpu_to_le16(0); + } + for (int j = 0; j < created_queues; j++) { + virtq_free(&data->virtqueues[j]); + } + k_free(data->virtqueues); + data->virtqueue_count = 0; + + return ret; } static bool virtio_pci_map_cap(pcie_bdf_t bdf, struct virtio_pci_cap *cap, void **virt_ptr)