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.
No know use of multiple device FDs is known. 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 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. This also makes the cdev path exclusive with group path.
This is done by adding a single_open flag in struct vfio_device_file and
a same flag in struct vfio_device_file. vfio_device_file::single_open is
set per the vfio_device_file allocation. Its value is propagated to struct
vfio_device after device is opened.
Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
---
drivers/vfio/group.c | 2 +-
drivers/vfio/vfio.h | 6 +++++-
drivers/vfio/vfio_main.c | 25 ++++++++++++++++++++++---
include/linux/vfio.h | 1 +
4 files changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
index c4d0564874f2..0b08a277cd0e 100644
--- a/drivers/vfio/group.c
+++ b/drivers/vfio/group.c
@@ -216,7 +216,7 @@ static struct file *vfio_device_open_file(struct vfio_device *device)
struct file *filep;
int ret;
- df = vfio_allocate_device_file(device);
+ df = vfio_allocate_device_file(device, false);
if (IS_ERR(df)) {
ret = PTR_ERR(df);
goto err_out;
diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index c099aa4e7d78..058d7a19dada 100644
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -17,7 +17,11 @@ struct vfio_device;
struct vfio_container;
struct vfio_device_file {
+ /* static fields, init per allocation */
struct vfio_device *device;
+ bool single_open;
+
+ /* fields set after allocation */
struct kvm *kvm;
struct iommufd_ctx *iommufd;
bool access_granted;
@@ -30,7 +34,7 @@ int vfio_device_open(struct vfio_device_file *df,
void vfio_device_close(struct vfio_device_file *device);
struct vfio_device_file *
-vfio_allocate_device_file(struct vfio_device *device);
+vfio_allocate_device_file(struct vfio_device *device, bool single_open);
extern const struct file_operations vfio_device_fops;
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 304633eee589..1bda847e9f10 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -359,7 +359,7 @@ static bool vfio_assert_device_open(struct vfio_device *device)
}
struct vfio_device_file *
-vfio_allocate_device_file(struct vfio_device *device)
+vfio_allocate_device_file(struct vfio_device *device, bool single_open)
{
struct vfio_device_file *df;
@@ -368,6 +368,7 @@ vfio_allocate_device_file(struct vfio_device *device)
return ERR_PTR(-ENOMEM);
df->device = device;
+ df->single_open = single_open;
return df;
}
@@ -435,6 +436,16 @@ 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. So a second device
+ * open attempt should be failed if the caller is from a cdev
+ * path or the device has already been opened by a cdev path.
+ */
+ if (device->open_count != 0 &&
+ (df->single_open || device->single_open))
+ return -EINVAL;
+
device->open_count++;
if (device->open_count == 1) {
int ret;
@@ -444,6 +455,7 @@ int vfio_device_open(struct vfio_device_file *df,
device->open_count--;
return ret;
}
+ device->single_open = df->single_open;
}
/*
@@ -465,8 +477,10 @@ void vfio_device_close(struct vfio_device_file *df)
*/
smp_store_release(&df->access_granted, false);
vfio_assert_device_open(device);
- if (device->open_count == 1)
+ if (device->open_count == 1) {
vfio_device_last_close(df);
+ device->single_open = false;
+ }
device->open_count--;
mutex_unlock(&device->dev_set->lock);
}
@@ -512,7 +526,12 @@ 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);
+ /*
+ * group path supports multiple device open, while cdev doesn't.
+ * So use vfio_device_group_close() for !singel_open case.
+ */
+ if (!df->single_open)
+ vfio_device_group_close(df);
kfree(df);
vfio_device_put_registration(device);
diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 9e3b9b5c8c8b..5465e29a8a83 100644
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -63,6 +63,7 @@ struct vfio_device {
struct iommufd_ctx *iommufd_ictx;
bool iommufd_attached;
#endif
+ bool single_open;
};
/**
--
2.34.1
