On Thu, May 19, 2022 at 06:43:06AM +0000, Tian, Kevin wrote: > > This fixes a user-triggerable oops in GVT. > > No changelog.
?? the cover latter clearly states what has changed since v1, and this patch has a good commit log. This is exactly how it is supposed to be done. > > > > > Signed-off-by: Jason Gunthorpe <[email protected]> > > Signed-off-by: Matthew Rosato <[email protected]> > > Not sure whether Christoph wants a s-o-b here when he wrote > the snippet to remove the release work of gvt... That's just tivial code removal, so no. > > > @@ -1083,11 +1083,22 @@ static struct file *vfio_device_open(struct > > vfio_device *device) > > > > mutex_lock(&device->dev_set->lock); > > device->open_count++; > > + down_read(&device->group->group_rwsem); > > + if (device->open_count == 1 && device->group->kvm) { > > + /* > > + * Here we pass the KVM pointer with the group under the > > read > > + * lock. If the device driver will use it, it must obtain a > > + * reference and release it during close_device. > > + */ > > + device->kvm = device->group->kvm; > > + } > > + > > if (device->open_count == 1 && device->ops->open_device) { > > Merge the two branches so both are under if (device->open_count == 1) {} > (and group_rwsem can be also moved inside) Yeah. And we don't really need the device->group->kvm check, as it would otherwise assign NULL which is perfectly fine. But otherwise this also looks good to me: Reviewed-by: Christoph Hellwig <[email protected]>
