When enabling MSI-X eventfds, a failure after creating some descriptors
leaves them open. The eventfd count is recorded only after all allocations
succeed, so disabling eventfds cannot clean up a partial allocation.

Calling enable again without disabling first also leaks descriptors by
overwriting the existing eventfd entries.

Reject repeated MSI-X enable calls with EBUSY before modifying the handle.
On failure, close all eventfds created by the call, clear their entries
and reset the eventfd count while preserving the original error.

Fixes: b9e0662a931d ("eal/linux: add interrupt API for drivers")
Cc: [email protected]

Signed-off-by: Anatoly Burakov <[email protected]>
---
 lib/eal/linux/eal_interrupts.c | 39 +++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 8 deletions(-)

diff --git a/lib/eal/linux/eal_interrupts.c b/lib/eal/linux/eal_interrupts.c
index 4cfaeba7fe0..ff05d649607 100644
--- a/lib/eal/linux/eal_interrupts.c
+++ b/lib/eal/linux/eal_interrupts.c
@@ -1570,30 +1570,43 @@ int
 rte_intr_efd_enable(struct rte_intr_handle *intr_handle, uint32_t nb_efd)
 {
        uint32_t i;
-       int fd;
+       int fd, ret;
        uint32_t n = RTE_MIN(nb_efd, (uint32_t)RTE_MAX_RXTX_INTR_VEC_ID);
 
        assert(nb_efd != 0);
 
        if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VFIO_MSIX) {
+               if (rte_intr_nb_efd_get(intr_handle) != 0) {
+                       EAL_LOG(ERR, "Interrupt eventfds are already enabled");
+                       return -EBUSY;
+               }
+
                for (i = 0; i < n; i++) {
                        fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
                        if (fd < 0) {
+                               ret = -errno;
                                EAL_LOG(ERR,
                                        "can't setup eventfd, error %i (%s)",
                                        errno, strerror(errno));
-                               return -errno;
+                               goto error;
                        }
 
-                       if (rte_intr_efds_index_set(intr_handle, i, fd))
-                               return -rte_errno;
+                       if (rte_intr_efds_index_set(intr_handle, i, fd)) {
+                               ret = -rte_errno;
+                               close(fd);
+                               goto error;
+                       }
                }
 
-               if (rte_intr_nb_efd_set(intr_handle, n))
-                       return -rte_errno;
+               if (rte_intr_nb_efd_set(intr_handle, n)) {
+                       ret = -rte_errno;
+                       goto error;
+               }
 
-               if (rte_intr_max_intr_set(intr_handle, NB_OTHER_INTR + n))
-                       return -rte_errno;
+               if (rte_intr_max_intr_set(intr_handle, NB_OTHER_INTR + n)) {
+                       ret = -rte_errno;
+                       goto error;
+               }
        } else if (rte_intr_type_get(intr_handle) == RTE_INTR_HANDLE_VDEV) {
                /* only check, initialization would be done in vdev driver.*/
                if ((uint64_t)rte_intr_efd_counter_size_get(intr_handle) >
@@ -1611,6 +1624,16 @@ rte_intr_efd_enable(struct rte_intr_handle *intr_handle, 
uint32_t nb_efd)
        }
 
        return 0;
+
+error:
+       while (i > 0) {
+               i--;
+               close(rte_intr_efds_index_get(intr_handle, i));
+               rte_intr_efds_index_set(intr_handle, i, -1);
+       }
+       rte_intr_nb_efd_set(intr_handle, 0);
+
+       return ret;
 }
 
 RTE_EXPORT_INTERNAL_SYMBOL(rte_intr_efd_disable)
-- 
2.52.0

Reply via email to