Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] vfio: NULL pointer dereference in vfio_df_iommufd_bind() via cdev ioctl - [High] vfio: NULL pointer deref when binding IOMMUFD to CPU_MMU cdev --
commit 147c0e7516a85b91b63d3c3d8f78088a9ca5c65f Author: Jean-Philippe Brucker <[email protected]> vfio: Support VFIO devices that access the CPU MMU Support devices that are attached to a CPU and perform DMA through a MMU without an IOMMU or VFIO container. The VMM uses device-specific knowledge to handle virtual device creation. > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index 423ead48aafe3..41fee81a14cbb 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -344,7 +344,8 @@ static int __vfio_register_dev(struct vfio_device *device, > { > int ret; > > - if (WARN_ON(IS_ENABLED(CONFIG_IOMMUFD) && > + if (type != VFIO_CPU_MMU && > + WARN_ON(IS_ENABLED(CONFIG_IOMMUFD) && > (!device->ops->bind_iommufd || > !device->ops->unbind_iommufd || > !device->ops->attach_ioas || > !device->ops->detach_ioas))) > return -EINVAL; [Severity: High] Could this bypass lead to a NULL pointer dereference for users of the character device interface? When CONFIG_VFIO_DEVICE_CDEV is enabled, vfio_device_add() automatically creates a character device node (e.g., /dev/vfio/devices/vfioX) for the registered device. If an untrusted user opens this cdev file and invokes the VFIO_DEVICE_BIND_IOMMUFD ioctl, the request routes through the following path: vfio_df_ioctl_bind_iommufd() vfio_df_open() vfio_df_device_first_open() vfio_df_iommufd_bind() Inside vfio_df_iommufd_bind() in drivers/vfio/iommufd.c, the code unconditionally executes: return vdev->ops->bind_iommufd(vdev, ictx, &df->devid); Since the updated check permits VFIO_CPU_MMU devices to be registered without providing a bind_iommufd callback, this would result in an immediate NULL pointer dereference. Similarly, calling the VFIO_DEVICE_ATTACH_IOMMUFD_PT ioctl on the cdev unconditionally dereferences the attach_ioas pointer in vfio_df_ioctl_attach_pt(), which would also crash the kernel. Should there be explicit checks in the cdev ioctl handlers to return -EOPNOTSUPP for devices lacking these callbacks, or should VFIO_CPU_MMU devices perhaps be excluded from cdev creation entirely? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=12
