[PATCH v1 1/1] eal/linux: fix eventfd leaks when enabling interrupts

[Burakov, Anatoly]

On 10/23/2026 1:42 PM, David Marchand wrote:
> Hello,
> 
> On Fri, 23 Oct 2026 at 11:37, Anatoly Burakov <anatoly.burakov at intel.com> 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: stable at dpdk.org
>>
>> Signed-off-by: Anatoly Burakov <anatoly.burakov at intel.com>
> 
> Good catch.
> How did you find this issue?

I didn't :D Bruce's AI review [1] caught the (somewhat confusingly 
formulated) bug, and I went on investigating.

[1] 
https://patches.dpdk.org/project/dpdk/patch/595c771fac2dda2d602c01ba171d56f3419db917.1792592724.git.anatoly.burakov@intel.com/#186792

> 
> I did not put warnings on the efd side, but maybe worth extending
> d770ee9caed0 ("interrupts: warn on leaked FDs").

Probably, yes.

> 
> 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'll see if this can be improved.

Thanks for the review!

> 
> 
>> +               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
>>
> 
> 


-- 
Thanks,
Anatoly