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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Out-of-bounds read of user memory and logic error in `vfio_ap_set_request_irq` and `vfio_ap_set_cfg_change_irq`.
--

commit 771c755d8bf563861fa80183f68a6dbe8966844c
Author: Anthony Krowiak <akrowiak@linux.ibm.com>

    s390/vfio-ap: Fix failure to release IRQ notification eventfd contexts
    
    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. These references
    are dropped only when userspace explicitly replaces or clears them,
    causing a leak if the device is closed without explicit teardown. This
    commit fixes the leak by introducing vfio_ap_mdev_release_eventfds()
    to clean up the references during close_device().

> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 9d4f5b9e3301f..4a31454d725a9 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c

[ ... ]

This isn't a bug introduced by this patch, but I noticed a pre-existing issue
in vfio_ap_set_request_irq() and vfio_ap_set_cfg_change_irq().

If a userspace process issues the VFIO_DEVICE_SET_IRQS ioctl with count == 0
and DATA_EVENTFD to clear the IRQ, could this unconditionally read out of
bounds?

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_set_request_irq() {
	s32 fd;
	void __user *data;
	unsigned long minsz;
...
	minsz = offsetofend(struct vfio_irq_set, count);
	data = (void __user *)(arg + minsz);

	if (get_user(fd, (s32 __user *)data))
		return -EFAULT;
...
}

[Severity: High]
Does this code read past the user-provided struct vfio_irq_set buffer if
count is 0?

Because there is no check for count > 0 before doing get_user(), it appears
this reads 4 bytes past the user-provided buffer and interprets the
uninitialized data as a file descriptor instead of safely disabling the
interrupt.

The same pattern appears in vfio_ap_set_cfg_change_irq():

drivers/s390/crypto/vfio_ap_ops.c:vfio_ap_set_cfg_change_irq() {
	s32 fd;
	void __user *data;
	unsigned long minsz;
...
	minsz = offsetofend(struct vfio_irq_set, count);
	data = (void __user *)(arg + minsz);

	if (get_user(fd, (s32 __user *)data))
		return -EFAULT;
...
}

[Severity: High]
Does this code also have the same out-of-bounds read issue when count is 0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904223531.1611088-1-akrowiak@linux.ibm.com?part=2