Re: [PATCH V4 05/10] vfio: Allow null group for noiommu without containers

Alex Williamson
2026-04-16T20:06:08Z

On Tue, 14 Apr 2026 14:14:07 -0700
Jacob Pan <jacob.pan@linux.microsoft.com> wrote:

> In case of noiommu mode is enabled for VFIO cdev without VFIO container
> nor IOMMUFD provided compatibility container, there is no need to
> create a dummy group. Update the group operations to tolerate null group
> pointer.
>
> Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
>
> ---
> v4: (Jason)
> - Avoid null pointer deref in error unwind
> - Add null group check in vfio_device_group_unregister
> - repartition to include vfio_device_has_group() in this patch
> ---
> drivers/vfio/group.c | 20 ++++++++++++++++++++
> drivers/vfio/vfio.h | 17 +++++++++++++++++
> drivers/vfio/vfio_main.c | 14 ++++++++++++++
> include/linux/vfio.h | 9 +++++++++
> 4 files changed, 60 insertions(+)
>
> diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> index 0fa9761b13d3..451e49d851f8 100644
> --- a/drivers/vfio/group.c
> +++ b/drivers/vfio/group.c
> @@ -390,6 +390,9 @@ int vfio_device_block_group(struct vfio_device *device)
> struct vfio_group *group = device->group;
> int ret = 0;
>
> + if (vfio_null_group_allowed() && !group)
> + return 0;

I think this comes down to the fact that at the end of this series,
VFIO_NOIOMMU still depends on VFIO_GROUP. vfio_null_group_allowed()
can only return true if CONTAINER support is entirely disabled. Why do
we still select VFIO_GROUP for VFIO_NOIOMMU and build group.s when
there's no container support to use it?

Also note that vfio_noiommu is S_IWUSR, so it is mutable at runtime.
Thanks,

Alex

> +
> mutex_lock(&group->group_lock);
> if (group->opened_file) {
> ret = -EBUSY;
> @@ -407,6 +410,9 @@ void vfio_device_unblock_group(struct vfio_device *device)
> {
> struct vfio_group *group = device->group;
>
> + if (vfio_null_group_allowed() && !group)
> + return;
> +
> mutex_lock(&group->group_lock);
> group->cdev_device_open_cnt--;
> mutex_unlock(&group->group_lock);
> @@ -598,6 +604,14 @@ static struct vfio_group *vfio_noiommu_group_alloc(struct device *dev,
> struct vfio_group *group;
> int ret;
>
> + /*
> + * With noiommu enabled under cdev interface only, there is no need to
> + * create a vfio_group if the group based containers are not enabled.
> + * The cdev interface is exclusively used for iommufd.
> + */
> + if (vfio_null_group_allowed())
> + return NULL;
> +
> iommu_group = iommu_group_alloc();
> if (IS_ERR(iommu_group))
> return ERR_CAST(iommu_group);
> @@ -705,6 +719,9 @@ void vfio_device_remove_group(struct vfio_device *device)
> struct vfio_group *group = device->group;
> struct iommu_group *iommu_group;
>
> + if (!group)
> + return;
> +
> if (group->type == VFIO_NO_IOMMU || group->type == VFIO_EMULATED_IOMMU)
> iommu_group_remove_device(device->dev);
>
> @@ -756,6 +773,9 @@ void vfio_device_group_register(struct vfio_device *device)
>
> void vfio_device_group_unregister(struct vfio_device *device)
> {
> + if (!device->group)
> + return;
> +
> mutex_lock(&device->group->device_lock);
> list_del(&device->group_next);
> mutex_unlock(&device->group->device_lock);
> diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
> index 8fcc98cf9577..db1530bb1716 100644
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -114,6 +114,18 @@ bool vfio_device_has_container(struct vfio_device *device);
> int __init vfio_group_init(void);
> void vfio_group_cleanup(void);
>
> +/*
> + * With noiommu enabled and no containers are supported, allow devices that
> + * don't have a dummy group.
> + */
> +static inline bool vfio_null_group_allowed(void)
> +{
> + if (vfio_noiommu && (!IS_ENABLED(CONFIG_VFIO_CONTAINER) && !IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)))
> + return true;
> +
> + return false;
> +}
> +
> static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> {
> return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> @@ -190,6 +202,11 @@ static inline void vfio_group_cleanup(void)
> {
> }
>
> +static inline bool vfio_null_group_allowed(void)
> +{
> + return false;
> +}
> +
> static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> {
> return false;
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index e5886235cad4..5d7c2d014689 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -358,6 +358,10 @@ static int __vfio_register_dev(struct vfio_device *device,
> /* Refcounting can't start until the driver calls register */
> refcount_set(&device->refcount, 1);
>
> + /* noiommu device w/o container may have NULL group */
> + if (!vfio_device_has_group(device))
> + return 0;
> +
> vfio_device_group_register(device);
> vfio_device_debugfs_init(device);
>
> @@ -392,6 +396,16 @@ void vfio_unregister_group_dev(struct vfio_device *device)
> bool interrupted = false;
> long rc;
>
> + /*
> + * For noiommu devices without a container, thus no dummy group,
> + * simply delete and unregister to balance refcount.
> + */
> + if (!vfio_device_has_group(device)) {
> + vfio_device_del(device);
> + vfio_device_put_registration(device);
> + return;
> + }
> +
> /*
> * Prevent new device opened by userspace via the
> * VFIO_GROUP_GET_DEVICE_FD in the group path.
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 7384965d15d7..ceb5034c3a2e 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -328,6 +328,10 @@ struct iommu_group *vfio_file_iommu_group(struct file *file);
> #if IS_ENABLED(CONFIG_VFIO_GROUP)
> bool vfio_file_is_group(struct file *file);
> bool vfio_file_has_dev(struct file *file, struct vfio_device *device);
> +static inline bool vfio_device_has_group(struct vfio_device *device)
> +{
> + return device->group;
> +}
> #else
> static inline bool vfio_file_is_group(struct file *file)
> {
> @@ -338,6 +342,11 @@ static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *devi
> {
> return false;
> }
> +
> +static inline bool vfio_device_has_group(struct vfio_device *device)
> +{
> + return false;
> +}
> #endif
> bool vfio_file_is_valid(struct file *file);
> bool vfio_file_enforced_coherent(struct file *file);