| Message ID | 20230316124156.12064-7-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introduce new methods for verifying ownership in vfio PCI hot reset | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Thursday, March 16, 2023 8:42 PM > > This extends both vfio_file_is_valid() and vfio_file_has_dev() to accept > device file from the vfio PCI hot reset. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Thu, Mar 16, 2023 at 05:41:55AM -0700, Yi Liu wrote: > This extends both vfio_file_is_valid() and vfio_file_has_dev() to accept > device file from the vfio PCI hot reset. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > drivers/vfio/vfio_main.c | 23 +++++++++++++++++++---- > 1 file changed, 19 insertions(+), 4 deletions(-) > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index fe7446805afd..ebbb6b91a498 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -1154,13 +1154,23 @@ const struct file_operations vfio_device_fops = { > .mmap = vfio_device_fops_mmap, > }; > > +static struct vfio_device *vfio_device_from_file(struct file *file) > +{ > + struct vfio_device *device = file->private_data; Isn't this a df now? > + if (file->f_op != &vfio_device_fops) > + return NULL; > + return device; > +} The device has to be bound to be a security proof. Jason
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Tuesday, March 21, 2023 3:08 AM > > On Thu, Mar 16, 2023 at 05:41:55AM -0700, Yi Liu wrote: > > This extends both vfio_file_is_valid() and vfio_file_has_dev() to accept > > device file from the vfio PCI hot reset. > > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > --- > > drivers/vfio/vfio_main.c | 23 +++++++++++++++++++---- > > 1 file changed, 19 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > > index fe7446805afd..ebbb6b91a498 100644 > > --- a/drivers/vfio/vfio_main.c > > +++ b/drivers/vfio/vfio_main.c > > @@ -1154,13 +1154,23 @@ const struct file_operations vfio_device_fops > = { > > .mmap = vfio_device_fops_mmap, > > }; > > > > +static struct vfio_device *vfio_device_from_file(struct file *file) > > +{ > > + struct vfio_device *device = file->private_data; > > Isn't this a df now? Not yet. It is placed before the cdev series. So it is vfio_device here. > > + if (file->f_op != &vfio_device_fops) > > + return NULL; > > + return device; > > +} > > The device has to be bound to be a security proof. I think it is because this helper is used by vfio_file_has_dev(). This requires to be bound to security proof. For now, the device fd is got via group. So as long s user can get it, it should have been bound. In the later cdev series, the below helper is added to ensure given device file has bound to security proof (a.k.a access_granted). +static bool vfio_file_has_device_access(struct file *file, + struct vfio_device *device) +{ + struct vfio_device *vdev = vfio_device_from_file(file); + struct vfio_device_file *df; + + if (!vdev || vdev != device) + return false; + + df = file->private_data; + + return READ_ONCE(df->access_granted); +} https://lore.kernel.org/kvm/20230316125534.17216-9-yi.l.liu@intel.com/ Regards, Yi Liu
On Thu, Mar 23, 2023 at 10:14:31AM +0000, Liu, Yi L wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Tuesday, March 21, 2023 3:08 AM > > > > On Thu, Mar 16, 2023 at 05:41:55AM -0700, Yi Liu wrote: > > > This extends both vfio_file_is_valid() and vfio_file_has_dev() to accept > > > device file from the vfio PCI hot reset. > > > > > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > > > --- > > > drivers/vfio/vfio_main.c | 23 +++++++++++++++++++---- > > > 1 file changed, 19 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > > > index fe7446805afd..ebbb6b91a498 100644 > > > --- a/drivers/vfio/vfio_main.c > > > +++ b/drivers/vfio/vfio_main.c > > > @@ -1154,13 +1154,23 @@ const struct file_operations vfio_device_fops > > = { > > > .mmap = vfio_device_fops_mmap, > > > }; > > > > > > +static struct vfio_device *vfio_device_from_file(struct file *file) > > > +{ > > > + struct vfio_device *device = file->private_data; > > > > Isn't this a df now? > > Not yet. It is placed before the cdev series. So it is vfio_device here. > > > > + if (file->f_op != &vfio_device_fops) > > > + return NULL; > > > + return device; > > > +} > > > > The device has to be bound to be a security proof. > > I think it is because this helper is used by vfio_file_has_dev(). This > requires to be bound to security proof. For now, the device fd is > got via group. So as long s user can get it, it should have been bound. > > In the later cdev series, the below helper is added to ensure > given device file has bound to security proof (a.k.a access_granted). Yes OK that makes senese Jason
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index fe7446805afd..ebbb6b91a498 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -1154,13 +1154,23 @@ const struct file_operations vfio_device_fops = { .mmap = vfio_device_fops_mmap, }; +static struct vfio_device *vfio_device_from_file(struct file *file) +{ + struct vfio_device *device = file->private_data; + + if (file->f_op != &vfio_device_fops) + return NULL; + return device; +} + /** * vfio_file_is_valid - True if the file is valid vfio file * @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); @@ -1174,12 +1184,17 @@ EXPORT_SYMBOL_GPL(vfio_file_is_valid); bool vfio_file_has_dev(struct file *file, struct vfio_device *device) { struct vfio_group *group; + struct vfio_device *vdev; group = vfio_group_from_file(file); - if (!group) - return false; + if (group) + return vfio_group_has_dev(group, device); + + vdev = vfio_device_from_file(file); + if (vdev) + return vdev == device; - return vfio_group_has_dev(group, device); + return false; } EXPORT_SYMBOL_GPL(vfio_file_has_dev);
This extends both vfio_file_is_valid() and vfio_file_has_dev() to accept device file from the vfio PCI hot reset. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/vfio/vfio_main.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)