Hello,
On Fri, 23 Oct 2026 at 11:37, Anatoly Burakov <[email protected]> wrote:
>
> 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]>
Good catch.
How did you find this issue?
I did not put warnings on the efd side, but maybe worth extending
d770ee9caed0 ("interrupts: warn on leaked FDs").
One comment below.
> ---
> 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) {
The i variable should have been declared only in the scope of VFIO
type, in the first place.
Using i here could blow up later when this error label is used for
other interrupt types.
As mentionned in the commitlog, could we accumulate the number of
valid efds in the vfio setup loop?
The cleanup code here could then loop on rte_intr_nb_efd_get.
> + 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
>
--
David Marchand
