Skip to content

Commit

Permalink
vfio: Remove calls to vfio_group_add_container_user()
Browse files Browse the repository at this point in the history 
When the open_device() op is called the container_users is incremented and
held incremented until close_device(). Thus, so long as drivers call
functions within their open_device()/close_device() region they do not
need to worry about the container_users.

These functions can all only be called between open_device() and
close_device():

  vfio_pin_pages()
  vfio_unpin_pages()
  vfio_dma_rw()
  vfio_register_notifier()
  vfio_unregister_notifier()

Eliminate the calls to vfio_group_add_container_user() and add
vfio_assert_device_open() to detect driver mis-use. This causes the
close_device() op to check device->open_count so always leave it elevated
while calling the op.

Reviewed-by: Christoph Hellwig <[email protected]>
Reviewed-by: Kevin Tian <[email protected]>
Signed-off-by: Jason Gunthorpe <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Alex Williamson <[email protected]>
  • Loading branch information
@jgunthorpe @awilliam
jgunthorpe authored and awilliam committed May 11, 2022
1 parent 231657b commit eadd86f
Showing 1 changed file with 17 additions and 63 deletions.
80 changes: 17 additions & 63 deletions drivers/vfio/vfio.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1330,6 +1330,12 @@ static int vfio_group_add_container_user(struct vfio_group *group)

static const struct file_operations vfio_device_fops;

/* true if the vfio_device has open_device() called but not close_device() */
static bool vfio_assert_device_open(struct vfio_device *device)
{
return !WARN_ON_ONCE(!READ_ONCE(device->open_count));
}

static int vfio_group_get_device_fd(struct vfio_group *group, char *buf)
{
struct vfio_device *device;
Expand Down Expand Up @@ -1544,8 +1550,10 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
struct vfio_device *device = filep->private_data;

mutex_lock(&device->dev_set->lock);
if (!--device->open_count && device->ops->close_device)
vfio_assert_device_open(device);
if (device->open_count == 1 && device->ops->close_device)
device->ops->close_device(device);
device->open_count--;
mutex_unlock(&device->dev_set->lock);

module_put(device->dev->driver->owner);
Expand Down Expand Up @@ -2112,7 +2120,8 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
struct vfio_iommu_driver *driver;
int ret;

if (!user_pfn || !phys_pfn || !npage)
if (!user_pfn || !phys_pfn || !npage ||
!vfio_assert_device_open(device))
return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
Expand All @@ -2121,10 +2130,6 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
if (group->dev_counter > 1)
return -EINVAL;

ret = vfio_group_add_container_user(group);
if (ret)
return ret;

container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->pin_pages))
Expand All @@ -2134,8 +2139,6 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
else
ret = -ENOTTY;

vfio_group_try_dissolve_container(group);

return ret;
}
EXPORT_SYMBOL(vfio_pin_pages);
Expand All @@ -2156,16 +2159,12 @@ int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
struct vfio_iommu_driver *driver;
int ret;

if (!user_pfn || !npage)
if (!user_pfn || !npage || !vfio_assert_device_open(device))
return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
return -E2BIG;

ret = vfio_group_add_container_user(device->group);
if (ret)
return ret;

container = device->group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unpin_pages))
Expand All @@ -2174,8 +2173,6 @@ int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
else
ret = -ENOTTY;

vfio_group_try_dissolve_container(device->group);

return ret;
}
EXPORT_SYMBOL(vfio_unpin_pages);
Expand Down Expand Up @@ -2204,13 +2201,9 @@ int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data,
struct vfio_iommu_driver *driver;
int ret = 0;

if (!data || len <= 0)
if (!data || len <= 0 || !vfio_assert_device_open(device))
return -EINVAL;

ret = vfio_group_add_container_user(device->group);
if (ret)
return ret;

container = device->group->container;
driver = container->iommu_driver;

Expand All @@ -2219,9 +2212,6 @@ int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data,
user_iova, data, len, write);
else
ret = -ENOTTY;

vfio_group_try_dissolve_container(device->group);

return ret;
}
EXPORT_SYMBOL(vfio_dma_rw);
Expand All @@ -2234,20 +2224,13 @@ static int vfio_register_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;

ret = vfio_group_add_container_user(group);
if (ret)
return -EINVAL;

container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->register_notifier))
ret = driver->ops->register_notifier(container->iommu_data,
events, nb);
else
ret = -ENOTTY;

vfio_group_try_dissolve_container(group);

return ret;
}

Expand All @@ -2258,20 +2241,13 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group,
struct vfio_iommu_driver *driver;
int ret;

ret = vfio_group_add_container_user(group);
if (ret)
return -EINVAL;

container = group->container;
driver = container->iommu_driver;
if (likely(driver && driver->ops->unregister_notifier))
ret = driver->ops->unregister_notifier(container->iommu_data,
nb);
else
ret = -ENOTTY;

vfio_group_try_dissolve_container(group);

return ret;
}

Expand Down Expand Up @@ -2300,10 +2276,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (*events)
return -EINVAL;

ret = vfio_group_add_container_user(group);
if (ret)
return -EINVAL;

ret = blocking_notifier_chain_register(&group->notifier, nb);

/*
Expand All @@ -2313,25 +2285,6 @@ static int vfio_register_group_notifier(struct vfio_group *group,
if (!ret && set_kvm && group->kvm)
blocking_notifier_call_chain(&group->notifier,
VFIO_GROUP_NOTIFY_SET_KVM, group->kvm);

vfio_group_try_dissolve_container(group);

return ret;
}

static int vfio_unregister_group_notifier(struct vfio_group *group,
struct notifier_block *nb)
{
int ret;

ret = vfio_group_add_container_user(group);
if (ret)
return -EINVAL;

ret = blocking_notifier_chain_unregister(&group->notifier, nb);

vfio_group_try_dissolve_container(group);

return ret;
}

Expand All @@ -2342,7 +2295,8 @@ int vfio_register_notifier(struct vfio_device *device,
struct vfio_group *group = device->group;
int ret;

if (!nb || !events || (*events == 0))
if (!nb || !events || (*events == 0) ||
!vfio_assert_device_open(device))
return -EINVAL;

switch (type) {
Expand All @@ -2366,15 +2320,15 @@ int vfio_unregister_notifier(struct vfio_device *device,
struct vfio_group *group = device->group;
int ret;

if (!nb)
if (!nb || !vfio_assert_device_open(device))
return -EINVAL;

switch (type) {
case VFIO_IOMMU_NOTIFY:
ret = vfio_unregister_iommu_notifier(group, nb);
break;
case VFIO_GROUP_NOTIFY:
ret = vfio_unregister_group_notifier(group, nb);
ret = blocking_notifier_chain_unregister(&group->notifier, nb);
break;
default:
ret = -EINVAL;
Expand Down

0 comments on commit eadd86f

Please sign in to comment.