On Tue, Sep 01, 2009 at 04:28:57PM +0100, Daniel P. Berrange wrote:
* src/security.h: Driver API for relabelling host devices * src/security_selinux.c: Implement relabelling of PCI and USB devices * src/qemu_driver.c: Relabel USB/PCI devices before hotplug --- src/qemu_driver.c | 12 ++- src/security.h | 7 ++ src/security_selinux.c | 175 +++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 174 insertions(+), 20 deletions(-)
diff --git a/src/qemu_driver.c b/src/qemu_driver.c index e9a09df..d75e28e 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -5498,6 +5498,9 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn,
if (qemuDomainSetDeviceOwnership(conn, driver, dev, 0) < 0) return -1; + if (driver->securityDriver && + driver->securityDriver->domainSetSecurityHostdevLabel(conn, vm, dev->data.hostdev) < 0) + return -1;
shouldn't we test against the entry point being null ? if (driver->securityDriver && driver->securityDriver->domainSetSecurityHostdevLabel && driver->securityDriver->domainSetSecurityHostdevLabel(conn, vm, dev->data.hostdev) < 0) Also the long line should probably be split. [...]
+ if (driver->securityDriver && + driver->securityDriver->domainSetSecurityHostdevLabel(conn, vm, dev->data.hostdev) < 0) + VIR_WARN0("Failed to restore device labelling"); +
idem [...]
static int +SELinuxRestoreSecurityImageLabel(virConnectPtr conn, + virDomainDiskDefPtr disk) +{ + /* Don't restore labels on readoly/shared disks, because
typo readonly
+ * other VMs may still be accessing these + * Alternatively we could iterate over all running + * domains and try to figure out if it is in use, but + * this would not work for clustered filesystems, since + * we can't see running VMs using the file on other nodes + * Safest bet is thus to skip the restore step. + */ + if (disk->readonly || disk->shared) + return 0; + + if (!disk->src) + return 0; + + return SELinuxRestoreSecurityFileLabel(conn, disk->src); +} + +static int SELinuxSetSecurityImageLabel(virConnectPtr conn, virDomainObjPtr vm, virDomainDiskDefPtr disk) @@ -414,6 +427,126 @@ SELinuxSetSecurityImageLabel(virConnectPtr conn, return 0; }
+ +static int +SELinuxSetSecurityPCILabel(virConnectPtr conn, + pciDevice *dev ATTRIBUTE_UNUSED, + const char *file, void *opaque) +{ + virDomainObjPtr vm = opaque; + const virSecurityLabelDefPtr secdef = &vm->def->seclabel; + + return SELinuxSetFilecon(conn, file, secdef->imagelabel); +}
it would be nice if we could check the type of the opaque passed in some ways, even if we know it was called a few line below.
+static int +SELinuxSetSecurityHostdevLabel(virConnectPtr conn, + virDomainObjPtr vm, + virDomainHostdevDefPtr dev) + +{ + int ret = -1; + + if (dev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + + switch (dev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + break; + + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: { + pciDevice *pci = pciGetDevice(conn, + dev->source.subsys.u.pci.domain, + dev->source.subsys.u.pci.bus, + dev->source.subsys.u.pci.slot, + dev->source.subsys.u.pci.function); + + if (!pci) + goto done; + + ret = pciDeviceFileIterate(conn, pci, SELinuxSetSecurityPCILabel, vm); + pciFreeDevice(conn, pci); + + break; + } + + default: + ret = 0; + break; + } + +done: + return ret; +}
Looks fine, ACK, but I won't pretend I understand all the implications of the security calls though, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/