[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
