lkml.org 
[lkml]   [2023]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3] vfio: fix potential deadlock on vfio group lock
From
On 1/13/23 1:52 PM, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2023 at 12:11:32PM -0500, Matthew Rosato wrote:
>> @@ -462,9 +520,19 @@ static inline void vfio_device_pm_runtime_put(struct vfio_device *device)
>> static int vfio_device_fops_release(struct inode *inode, struct file *filep)
>> {
>> struct vfio_device *device = filep->private_data;
>> + struct kvm *kvm = NULL;
>>
>> vfio_device_group_close(device);
>>
>> + mutex_lock(&device->dev_set->lock);
>> + if (device->open_count == 0 && device->kvm) {
>> + kvm = device->kvm;
>> + device->kvm = NULL;
>> + }
>> + mutex_unlock(&device->dev_set->lock);
>
> This still doesn't seem right, another thread could have incr'd the
> open_count already
>
> This has to be done at the moment open_count is decremented to zero,
> while still under the original lock.

Hmm.. Fair. Well, we can go back to clearing of device->kvm in vfio_device_last_close but the group lock is held then so we can't immediately do the kvm_put at that time -- unless we go back to the notion of stacking the kvm_put on a workqueue, but now from vfio. If we do that, I think we also have to scrap the idea of putting the kvm_put_kvm function pointer into device->put_kvm too (or otherwise stash it along with the kvm value to be picked up by the scheduled work).

Another thought would be passing the device->open_count that was read while holding the dev_set->lock back on vfio_close_device() / vfio_device_group_close() as an indicator of whether vfio_device_last_close() was called - then you could use the stashed kvm value because it doesn't matter what's currently in device->kvm or what the current device->open_count is, you know that kvm reference needs to be put.

e.g.:
struct *kvm = device->kvm;
void (*put)(struct kvm *kvm) = device->put_kvm;
opened = vfio_device_group_close(device);
if (opened == 0 && kvm)
put(kvm);

\
 
 \ /
  Last update: 2023-03-26 23:41    [W:0.130 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site