On Mon, 10 Jun 2024 21:53:05 +0100 Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote: > On Mon, Jun 10, 2024 at 02:09:06PM -0600, Alex Williamson wrote: > > > > > > We could move vfs_poll() under vm->irqfds_lock, but that smells > > > like asking for deadlocks ;-/ > > > > > > vfio_virqfd_enable() has the same problem, except that there we > > > definitely can't move vfs_poll() under the lock - it's a spinlock. > > > > vfio_virqfd_enable() and vfio_virqfd_disable() are serialized by their > > callers, I don't see that they have a UAF problem. Thanks, > > > > Alex > > Umm... I agree that there's no UAF on vfio side; acrn and xen/privcmd > counterparts, OTOH, look like they do have that... > > OK, so the memory safety in there depends upon > * external exclusion wrt vfio_virqfd_disable() on caller-specific > locks (vfio_pci_core_device::ioeventfds_lock for vfio_pci_rdwr.c, > vfio_pci_core_device::igate for the rest? What about the path via > vfio_pci_core_disable()?) This is only called when the device is closed, therefore there's no userspace access to generate a race. > * no EPOLLHUP on eventfd while the file is pinned. That's what > /* > * Do not drop the file until the irqfd is fully initialized, > * otherwise we might race against the EPOLLHUP. > */ > in there (that "irqfd" is a typo for "kirqfd", right?) refers to. Sorry, I'm not fully grasping your comment. "irqfd" is not a typo here, "kirqfd" seems to be a Xen thing. I believe the comment is referring to holding a reference to the fd until everything is in place to cleanup correctly if the user process is killed mid-setup. Thanks, Alex
