Re: [RFC PATCH v2 12/15] vfio: Support VFIO devices that access the CPU MMU

[sashiko-bot@kernel.org]

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 <jpb@kernel.org>

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/20261015142552.4128028-1-ryan.roberts@arm.com?part=12