| Message ID | 20221110014027.28780-2-ajderossi@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/pci: Check the device set open count on reset | expand |
On 2022/11/10 09:40, Anthony DeRossi wrote: > In vfio_device_open(), vfio_device_container_register() is always called > when open_count == 1. On error, vfio_device_container_unregister() is > only called when open_count == 1 and close_device is set. This leaks a > registration for devices without a close_device implementation. > > In vfio_device_fops_release(), vfio_device_container_unregister() is > called unconditionally. This can cause a device to be unregistered > multiple times. > > Treating container device registration/unregistration uniformly (always > when open_count == 1) fixes both issues. > > Fixes: ce4b4657ff18 ("vfio: Replace the DMA unmapping notifier with a callback") > Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > Reviewed-by: Kevin Tian <kevin.tian@intel.com> > --- > drivers/vfio/vfio_main.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) Reviewed-by: Yi Liu <yi.l.liu@intel.com> > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index 2d168793d4e1..9a4af880e941 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -801,8 +801,9 @@ static struct file *vfio_device_open(struct vfio_device *device) > err_close_device: > mutex_lock(&device->dev_set->lock); > mutex_lock(&device->group->group_lock); > - if (device->open_count == 1 && device->ops->close_device) { > - device->ops->close_device(device); > + if (device->open_count == 1) { > + if (device->ops->close_device) > + device->ops->close_device(device); > > vfio_device_container_unregister(device); > } > @@ -1017,10 +1018,12 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) > mutex_lock(&device->dev_set->lock); > vfio_assert_device_open(device); > mutex_lock(&device->group->group_lock); > - if (device->open_count == 1 && device->ops->close_device) > - device->ops->close_device(device); > + if (device->open_count == 1) { > + if (device->ops->close_device) > + device->ops->close_device(device); > > - vfio_device_container_unregister(device); > + vfio_device_container_unregister(device); > + } > mutex_unlock(&device->group->group_lock); > device->open_count--; > if (device->open_count == 0)
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 2d168793d4e1..9a4af880e941 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -801,8 +801,9 @@ static struct file *vfio_device_open(struct vfio_device *device) err_close_device: mutex_lock(&device->dev_set->lock); mutex_lock(&device->group->group_lock); - if (device->open_count == 1 && device->ops->close_device) { - device->ops->close_device(device); + if (device->open_count == 1) { + if (device->ops->close_device) + device->ops->close_device(device); vfio_device_container_unregister(device); } @@ -1017,10 +1018,12 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) mutex_lock(&device->dev_set->lock); vfio_assert_device_open(device); mutex_lock(&device->group->group_lock); - if (device->open_count == 1 && device->ops->close_device) - device->ops->close_device(device); + if (device->open_count == 1) { + if (device->ops->close_device) + device->ops->close_device(device); - vfio_device_container_unregister(device); + vfio_device_container_unregister(device); + } mutex_unlock(&device->group->group_lock); device->open_count--; if (device->open_count == 0)
In vfio_device_open(), vfio_device_container_register() is always called when open_count == 1. On error, vfio_device_container_unregister() is only called when open_count == 1 and close_device is set. This leaks a registration for devices without a close_device implementation. In vfio_device_fops_release(), vfio_device_container_unregister() is called unconditionally. This can cause a device to be unregistered multiple times. Treating container device registration/unregistration uniformly (always when open_count == 1) fixes both issues. Fixes: ce4b4657ff18 ("vfio: Replace the DMA unmapping notifier with a callback") Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com> --- drivers/vfio/vfio_main.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-)