From 2e4c1b50d4a3fbfb5f59fdbeed6e10c1d3761cc2 Mon Sep 17 00:00:00 2001 From: Maxime Coquelin Date: Tue, 26 Jan 2021 11:16:39 +0100 Subject: [PATCH] net/virtio: handle virtio-user setup failure properly This patch fixes virtio_user_dev_setup() error path, by cleaning all resources it allocates. It introduces virtio_user_dev_uninit_notify() that cleans all open FDs. It implies assigning all FDs to -1 at init time. With these changes done, virtio_user_dev_init_notify() can be simplified. Suggested-by: Adrian Moreno Signed-off-by: Maxime Coquelin Reviewed-by: Chenbo Xia --- .../net/virtio/virtio_user/virtio_user_dev.c | 70 +++++++++++++------ 1 file changed, 47 insertions(+), 23 deletions(-) diff --git a/drivers/net/virtio/virtio_user/virtio_user_dev.c b/drivers/net/virtio/virtio_user/virtio_user_dev.c index a1e32158bb..1b54d55bd8 100644 --- a/drivers/net/virtio/virtio_user/virtio_user_dev.c +++ b/drivers/net/virtio/virtio_user/virtio_user_dev.c @@ -283,13 +283,7 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev) int callfd; int kickfd; - for (i = 0; i < VIRTIO_MAX_VIRTQUEUES; ++i) { - if (i >= dev->max_queue_pairs * 2) { - dev->kickfds[i] = -1; - dev->callfds[i] = -1; - continue; - } - + for (i = 0; i < dev->max_queue_pairs * 2; i++) { /* May use invalid flag, but some backend uses kickfd and * callfd as criteria to judge if dev is alive. so finally we * use real event_fd. @@ -297,28 +291,49 @@ virtio_user_dev_init_notify(struct virtio_user_dev *dev) callfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); if (callfd < 0) { PMD_DRV_LOG(ERR, "(%s) callfd error, %s", dev->path, strerror(errno)); - break; + goto err; } kickfd = eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK); if (kickfd < 0) { close(callfd); PMD_DRV_LOG(ERR, "(%s) kickfd error, %s", dev->path, strerror(errno)); - break; + goto err; } dev->callfds[i] = callfd; dev->kickfds[i] = kickfd; } - if (i < VIRTIO_MAX_VIRTQUEUES) { - for (j = 0; j < i; ++j) { - close(dev->callfds[j]); + return 0; +err: + for (j = 0; j < i; j++) { + if (dev->kickfds[j] >= 0) { close(dev->kickfds[j]); + dev->kickfds[j] = -1; + } + if (dev->callfds[j] >= 0) { + close(dev->callfds[j]); + dev->callfds[j] = -1; } - - return -1; } - return 0; + return -1; +} + +static void +virtio_user_dev_uninit_notify(struct virtio_user_dev *dev) +{ + uint32_t i; + + for (i = 0; i < dev->max_queue_pairs * 2; ++i) { + if (dev->kickfds[i] >= 0) { + close(dev->kickfds[i]); + dev->kickfds[i] = -1; + } + if (dev->callfds[i] >= 0) { + close(dev->callfds[i]); + dev->callfds[i] = -1; + } + } } static int @@ -427,15 +442,22 @@ virtio_user_dev_setup(struct virtio_user_dev *dev) if (virtio_user_dev_init_notify(dev) < 0) { PMD_INIT_LOG(ERR, "(%s) Failed to init notifiers\n", dev->path); - return -1; + goto destroy; } if (virtio_user_fill_intr_handle(dev) < 0) { PMD_INIT_LOG(ERR, "(%s) Failed to init interrupt handler\n", dev->path); - return -1; + goto uninit; } return 0; + +uninit: + virtio_user_dev_uninit_notify(dev); +destroy: + dev->ops->destroy(dev); + + return -1; } /* Use below macro to filter features from vhost backend */ @@ -466,9 +488,16 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, enum virtio_user_backend_type backend_type) { uint64_t backend_features; + int i; pthread_mutex_init(&dev->mutex, NULL); strlcpy(dev->path, path, PATH_MAX); + + for (i = 0; i < VIRTIO_MAX_VIRTQUEUES; i++) { + dev->kickfds[i] = -1; + dev->callfds[i] = -1; + } + dev->started = 0; dev->max_queue_pairs = queues; dev->queue_pairs = 1; /* mq disabled by default */ @@ -565,16 +594,11 @@ virtio_user_dev_init(struct virtio_user_dev *dev, char *path, int queues, void virtio_user_dev_uninit(struct virtio_user_dev *dev) { - uint32_t i; - virtio_user_stop_device(dev); rte_mem_event_callback_unregister(VIRTIO_USER_MEM_EVENT_CLB_NAME, dev); - for (i = 0; i < dev->max_queue_pairs * 2; ++i) { - close(dev->callfds[i]); - close(dev->kickfds[i]); - } + virtio_user_dev_uninit_notify(dev); free(dev->ifname); -- 2.20.1