[Cc +Paolo] On Wed, 15 Feb 2023 10:46:34 -0400 Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > On Wed, Feb 15, 2023 at 02:43:20PM +0000, Liu, Yi L wrote: > > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > > Sent: Wednesday, February 15, 2023 8:39 PM > > > > > > On Tue, Feb 14, 2023 at 02:02:37AM +0000, Liu, Yi L wrote: > > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > > > > Sent: Tuesday, February 14, 2023 7:44 AM > > > > > > > > > > On Mon, Feb 13, 2023 at 07:13:36AM -0800, Yi Liu wrote: > > > > > > +static struct vfio_device *vfio_device_from_file(struct file *file) > > > > > > +{ > > > > > > + struct vfio_device_file *df = file->private_data; > > > > > > + > > > > > > + if (file->f_op != &vfio_device_fops) > > > > > > + return NULL; > > > > > > + return df->device; > > > > > > +} > > > > > > + > > > > > > /** > > > > > > * vfio_file_is_valid - True if the file is usable with VFIO APIS > > > > > > * @file: VFIO group file or VFIO device file > > > > > > */ > > > > > > bool vfio_file_is_valid(struct file *file) > > > > > > { > > > > > > - return vfio_group_from_file(file); > > > > > > + return vfio_group_from_file(file) || > > > > > > + vfio_device_from_file(file); > > > > > > } > > > > > > EXPORT_SYMBOL_GPL(vfio_file_is_valid); > > > > > > > > > > This can only succeed on a device cdev that has been fully opened. > > > > > > > > Actually, we cannot. This is used in the kvm-vfio code to see if the > > > > user-provided fd is vfio fds in the SET_KVM path. And we don't > > > > have the device cdev fully opened until BIND_IOMMUFD. But we do > > > > need to invoke SET_KVM before issuing BIND_IOMMUFD as the device > > > > open needs kvm pointer. So if we cannot apply fully opened limit to this > > > > interface. Maybe an updated function comment is needed. > > > > > > This also seems sketchy, KVM is using the VFIO fd as a "proof" to > > > enable the wbinvd stuff. A half opened cdev should not be used as that > > > proof. > > > > From this angle, the group path seems has the same concern. Device is not > > opened until VFIO_GROUP_GET_DEVICE_FD. > > Well, classically the device was DMA ownership claimed at least. > > > But group path has one advantage, which make it ok. Group can only be > > opened by one application. So once it is opened, the devices within the > > group are somehow obtained by the application until group fd close. > > It depends on what do we want the KVM proof to actually mean. > > Is simply having permissions on the cdev node sufficient proof for > wbinvd? > > I admit I poorly understand the threat model for this in kvm beyond > that kvm doesn't want everyone to use wbinvd. We've discussed this with Paolo before and I believe the bar of proof is not very high. I suspect it's not a problem that the device itself is not yet accessible, so long as the user can prove they have the ability to access the device, such as access to a restricted file. In most cases this isn't going to turn on wbinvd anyway since DMA will be coherent. Thanks, Alex