[libvirt] [PATCH 3/8] qemu: add VFIO devices to cgroup ACL

2013-04-25 Thread Laine Stump 
We don't know exactly the names of the VFIO devices that will be
needed (and due to hotplug, we can't ever assume we won't need them at
all), so we just add an ACL to allow any vfio device - they all have
the major number 244 (/dev/vfio/vfio is 244,0, and the /dev/vfio/n
devices are up from there).
---
 src/qemu/qemu_cgroup.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 891984a..ad2027d 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -44,6 +44,7 @@ static const char *const defaultDeviceACL[] = {
 };
 #define DEVICE_PTY_MAJOR 136
 #define DEVICE_SND_MAJOR 116
+#define DEVICE_VFIO_MAJOR 244
 
 static int
 qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
@@ -388,6 +389,16 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
 }
 }
 
+rc = virCgroupAllowDeviceMajor(priv-cgroup, 'c', DEVICE_VFIO_MAJOR,
+   VIR_CGROUP_DEVICE_RW);
+virDomainAuditCgroupMajor(vm, priv-cgroup, allow, DEVICE_VFIO_MAJOR,
+  vfio, rw, rc == 0);
+if (rc != 0) {
+virReportSystemError(-rc, %s,
+ _(unable to allow /dev/vfio/ devices));
+goto cleanup;
+}
+
 for (i = 0; deviceACL[i] != NULL ; i++) {
 if (access(deviceACL[i], F_OK)  0) {
 VIR_DEBUG(Ignoring non-existant device %s,
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/8] qemu: add VFIO devices to cgroup ACL

2013-04-25 Thread Eric Blake 
On 04/25/2013 11:57 AM, Laine Stump wrote:
 We don't know exactly the names of the VFIO devices that will be
 needed (and due to hotplug, we can't ever assume we won't need them at
 all), so we just add an ACL to allow any vfio device - they all have
 the major number 244 (/dev/vfio/vfio is 244,0, and the /dev/vfio/n
 devices are up from there).
 ---
  src/qemu/qemu_cgroup.c | 11 +++
  1 file changed, 11 insertions(+)

 @@ -388,6 +389,16 @@ int qemuSetupCgroup(virQEMUDriverPtr driver,
  }
  }
  
 +rc = virCgroupAllowDeviceMajor(priv-cgroup, 'c', DEVICE_VFIO_MAJOR,
 +   VIR_CGROUP_DEVICE_RW);
 +virDomainAuditCgroupMajor(vm, priv-cgroup, allow, 
 DEVICE_VFIO_MAJOR,
 +  vfio, rw, rc == 0);
 +if (rc != 0) {
 +virReportSystemError(-rc, %s,
 + _(unable to allow /dev/vfio/ devices));
 +goto cleanup;
 +}

This matches what we have done for ptys and sound; but it's rather broad
in that the access is now ALWAYS allowed.  For ptys, always being
enabled makes sense (you seldom start a guest without a pty, and even if
we blindly grant the privilege to a guest that doesn't use a pty there's
not much they could break by a rogue guest opening a pty device); but
for vfio, it's tied to something that we don't want to allow rogue
access to, when it can be avoided.

Do we want to be a bit smarter and only add it to the file at the point
when we start a guest with a vfio device and/or hotplug a vfio device,
then revoke the cgroup privileges any time we hot-unplug the last vfio
device?  If so, it would be similar to how we add cgroup device ACL
privileges for block disks at hotplug time, except that we'd need some
domain tracking to count how many vfio devices are plugged at once, so
we aren't revoking access until the last hot-unplug.

I can live with this as-is (we at least audit that it happens), but
wonder if it is worth respinning this patch to be smarter about when to
allow vfio in the guest.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list