Re: [libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
On Thu, Apr 25, 2013 at 09:44:33PM -0400, 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). We do the correct labelling of the /dev/vfio/N device in the security drivers, so we should be able todo the same for cgroups device ACL. Allowing all N is not acceptable from a security POV. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
On 04/26/2013 04:52 AM, Daniel P. Berrange wrote: On Thu, Apr 25, 2013 at 09:44:33PM -0400, 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). We do the correct labelling of the /dev/vfio/N device in the security drivers, so we should be able todo the same for cgroups device ACL. Allowing all N is not acceptable from a security POV. Up until now there hasn't been any cgroup-related code in the security drivers, though. So where should this go? Do we need a new driver backend for cgroups? This would then mean that we need to have three tiers of security drivers rather than two. Or can it just be put in the DAC driver? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
On Fri, Apr 26, 2013 at 11:16:14AM -0400, Laine Stump wrote: On 04/26/2013 04:52 AM, Daniel P. Berrange wrote: On Thu, Apr 25, 2013 at 09:44:33PM -0400, 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). We do the correct labelling of the /dev/vfio/N device in the security drivers, so we should be able todo the same for cgroups device ACL. Allowing all N is not acceptable from a security POV. Up until now there hasn't been any cgroup-related code in the security drivers, though. So where should this go? Do we need a new driver backend for cgroups? This would then mean that we need to have three tiers of security drivers rather than two. Or can it just be put in the DAC driver? We manage perfectly well to configure ACLs for individual disks that a VM is given without having to wildcard allow every single /dev/sdN disk. That fact that you were able to make the security drivers label the /dev/vfio/n devices correctly, shows that the information required is available. So why can't you set the cgroups ACLs correctly here too ? There's no need to move cgroups code into any security driver. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
On 04/26/2013 11:42 AM, Daniel P. Berrange wrote: On Fri, Apr 26, 2013 at 11:16:14AM -0400, Laine Stump wrote: On 04/26/2013 04:52 AM, Daniel P. Berrange wrote: On Thu, Apr 25, 2013 at 09:44:33PM -0400, 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). We do the correct labelling of the /dev/vfio/N device in the security drivers, so we should be able todo the same for cgroups device ACL. Allowing all N is not acceptable from a security POV. Up until now there hasn't been any cgroup-related code in the security drivers, though. So where should this go? Do we need a new driver backend for cgroups? This would then mean that we need to have three tiers of security drivers rather than two. Or can it just be put in the DAC driver? We manage perfectly well to configure ACLs for individual disks that a VM is given without having to wildcard allow every single /dev/sdN disk. That fact that you were able to make the security drivers label the /dev/vfio/n devices correctly, shows that the information required is available. So why can't you set the cgroups ACLs correctly here too ? There's no need to move cgroups code into any security driver. Sorry, my brain combined the first and second sentences of your message, and understood that you wanted this to happen in the security driver. I'll look up what's done for disks. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
On 04/26/2013 09:55 AM, Laine Stump wrote: We manage perfectly well to configure ACLs for individual disks that a VM is given without having to wildcard allow every single /dev/sdN disk. That fact that you were able to make the security drivers label the /dev/vfio/n devices correctly, shows that the information required is available. So why can't you set the cgroups ACLs correctly here too ? There's no need to move cgroups code into any security driver. Sorry, my brain combined the first and second sentences of your message, and understood that you wanted this to happen in the security driver. I'll look up what's done for disks. Basically, we have code that does four related things - call into the security manager, call into the cgroup manager, call into the lock space manager, and finally audit the result. See qemuDomainPrepareDiskChainElement for an example. -- 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
[libvirt] [PATCH 4/4] qemu: add VFIO devices to cgroup ACL
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