Re: [PATCH 2/4] s390/vfio-ap: Fix failure to release IRQ notification eventfd contexts

On 8/24/26 9:58 AM, Anthony Krowiak wrote:
> When userspace registers IRQ notification eventfds via the
> VFIO_DEVICE_SET_IRQS ioctl, vfio_ap_set_request_irq() and
> vfio_ap_set_cfg_change_irq() each call eventfd_ctx_fdget(), which
> takes a reference on the eventfd_ctx and stores it in
> matrix_mdev->req_trigger and matrix_mdev->cfg_chg_trigger
> respectively.
> 
> These references are dropped only when userspace explicitly replaces
> or clears them via a subsequent SET_IRQS call.  If the device is
> closed without that explicit teardown - because the guest exits,
> the VM process crashes, or the device file is simply closed -
> neither vfio_ap_mdev_close_device() nor the remove path releases
> these references.  The eventfd_ctx backing objects and their
> associated file references therefore leak for the lifetime of the
> kernel.
> 
> Fix this by introducing vfio_ap_mdev_release_eventfds() and calling
> it from vfio_ap_mdev_close_device() after vfio_ap_mdev_unset_kvm().
> The VFIO core guarantees that close_device is called before
> vfio_unregister_group_dev() returns in the remove path, so fixing
> close_device is sufficient to cover both teardown paths.
> 
> Note:
> ~~~~
> The matrix_dev->mdevs lock must be held during the call to
> vfio_ap_mdev_release_eventfds(). There is a small window between the calls
> to vfio_ap_mdev_unset_kvm() which gets and releases the update locks
> and the acquisition of the matrix_dev->mdevs_lock mutex during which
> it is possible - although highly unlikely during normal operation - whereby
> a concurrent SET_IRQS call can get in.
> 
> Taking matrix_dev->mdevs_lock around vfio_ap_mdev_release_eventfds()
> is sufficient to make this race-free. The SET_IRQS ioctl path writes
> req_trigger and cfg_chg_trigger only from vfio_ap_mdev_ioctl(), which
> holds mdevs_lock for its entire duration and always calls
> eventfd_ctx_put() on the previous value before storing the new one.
> 
> Any number of concurrent SET_IRQS calls during the window between
> vfio_ap_mdev_unset_kvm() and the acquisition of mdevs_lock are
> therefore safe: each ioctl invocation puts the reference it found and
> installs a new one, leaving exactly one live reference in the field
> when it releases the lock. When release_eventfds subsequently acquires
> mdevs_lock it finds that single surviving reference and puts it.
> Conversely, a SET_IRQS call that loses the race and blocks on
> mdevs_lock will find the field NULL after release_eventfds finishes,
> take ownership of the reference it just created, and install it into a
> field that will never be read again - a transient leak. To close that
> final case, callers must ensure no new SET_IRQS ioctls can be issued
> after close_device() is called, which the VFIO core guarantees by
> releasing the device file before invoking close_device().
> 
> Fixes: bf48961f6f48e ("s390/vfio-ap: realize the VFIO_DEVICE_SET_IRQS ioctl")
> Cc: [email protected]
> Signed-off-by: Anthony Krowiak <[email protected]>

Reviewed-by: Matthew Rosato <[email protected]>