| Message ID | 20230221034812.138051-15-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add vfio_device cdev for iommufd support | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Tuesday, February 21, 2023 11:48 AM > > VFIO group has historically allowed multi-open of the device FD. This > was made secure because the "open" was executed via an ioctl to the > group FD which is itself only single open. > > However, no known use of multiple device FDs today. It is kind of a > strange thing to do because new device FDs can naturally be created > via dup(). > > When we implement the new device uAPI (only used in cdev path) there is > no natural way to allow the device itself from being multi-opened in a > secure manner. Without the group FD we cannot prove the security context > of the opener. > > Thus, when moving to the new uAPI we block the ability to multi-open > the device. Old group path still allows it. > > vfio_device_open() needs to sustain both the legacy behavior i.e. multi-open > in the group path and the new behavior i.e. single-open in the cdev path. > This mixture leads to the introduction of a new is_cdev_device flag in struct > vfio_device_file. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h index bf84cf36eac7..68be0e8279c7 100644 --- a/drivers/vfio/vfio.h +++ b/drivers/vfio/vfio.h @@ -18,6 +18,8 @@ struct vfio_container; struct vfio_device_file { struct vfio_device *device; + bool is_cdev_device; + bool access_granted; spinlock_t kvm_ref_lock; /* protect kvm field */ struct kvm *kvm; diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 484f89eef7e5..925127a38a3a 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -472,6 +472,13 @@ int vfio_device_open(struct vfio_device_file *df, lockdep_assert_held(&device->dev_set->lock); + /* + * Device cdev path cannot support multiple device open since + * it doesn't have a secure way for it. + */ + if (device->open_count != 0 && df->is_cdev_device) + return -EINVAL; + device->open_count++; if (device->open_count == 1) { ret = vfio_device_first_open(df, dev_id, pt_id); @@ -535,7 +542,8 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) struct vfio_device_file *df = filep->private_data; struct vfio_device *device = df->device; - vfio_device_group_close(df); + if (!df->is_cdev_device) + vfio_device_group_close(df); vfio_device_put_registration(device);
VFIO group has historically allowed multi-open of the device FD. This was made secure because the "open" was executed via an ioctl to the group FD which is itself only single open. However, no known use of multiple device FDs today. It is kind of a strange thing to do because new device FDs can naturally be created via dup(). When we implement the new device uAPI (only used in cdev path) there is no natural way to allow the device itself from being multi-opened in a secure manner. Without the group FD we cannot prove the security context of the opener. Thus, when moving to the new uAPI we block the ability to multi-open the device. Old group path still allows it. vfio_device_open() needs to sustain both the legacy behavior i.e. multi-open in the group path and the new behavior i.e. single-open in the cdev path. This mixture leads to the introduction of a new is_cdev_device flag in struct vfio_device_file. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- drivers/vfio/vfio.h | 2 ++ drivers/vfio/vfio_main.c | 10 +++++++++- 2 files changed, 11 insertions(+), 1 deletion(-)