On Tue, Sep 08, 2009 at 04:19:59PM -0500, Jamie Strandboge wrote:
On Tue, 08 Sep 2009, Jamie Strandboge wrote:
[PATCH 1*] patch_1a_reenable-nonfile-labels.patch: When James Morris originally submitted his sVirt patches (as seen in libvirt 0.6.1), he did not require on disk labelling for virSecurityDomainRestoreImageLabel. A later commit[2] changed this behavior to assume on disk labelling, which halts implementations for path-based MAC systems such as AppArmor and TOMOYO where vm->def->seclabel is required to obtain the label. This patch simply adds the 'virDomainObjPtr vm' argument back to *RestoreImageLabel.
patch_1b_optional.patch: Due to the above change, 'make syntax-check' fails because SELinuxRestoreSecurityImageLabel() does not use the 'virDomainObjPtr vm'. patch_1b_optional.patch is a simple patch to fix this by checking if vm->def->seclabel == NULL and returns with error if it does. I realize this may not be desired in the long term, but it should be harmless enough to include.
The 'ATTRIBUTE_UNUSED' anotation takes care of this much better.
diff -Nurp ./libvirt.orig/src/qemu_driver.c ./libvirt/src/qemu_driver.c --- ./libvirt.orig/src/qemu_driver.c 2009-09-08 13:00:00.000000000 -0500 +++ ./libvirt/src/qemu_driver.c 2009-09-08 15:32:22.000000000 -0500 @@ -6144,7 +6144,7 @@ static int qemudDomainDetachDevice(virDo dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_VIRTIO)) { ret = qemudDomainDetachPciDiskDevice(dom->conn, vm, dev); if (driver->securityDriver) - driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, dev->data.disk); + driver->securityDriver->domainRestoreSecurityImageLabel(dom->conn, vm, dev->data.disk); if (qemuDomainSetDeviceOwnership(dom->conn, driver, dev, 1) < 0) VIR_WARN0("Fail to restore disk device ownership"); } else if (dev->type == VIR_DOMAIN_DEVICE_NET) { diff -Nurp ./libvirt.orig/src/security.h ./libvirt/src/security.h --- ./libvirt.orig/src/security.h 2009-08-17 11:00:40.000000000 -0500 +++ ./libvirt/src/security.h 2009-09-08 15:32:22.000000000 -0500 @@ -32,6 +32,7 @@ typedef virSecurityDriverStatus (*virSec typedef int (*virSecurityDriverOpen) (virConnectPtr conn, virSecurityDriverPtr drv); typedef int (*virSecurityDomainRestoreImageLabel) (virConnectPtr conn, + virDomainObjPtr vm, virDomainDiskDefPtr disk); typedef int (*virSecurityDomainSetImageLabel) (virConnectPtr conn, virDomainObjPtr vm, diff -Nurp ./libvirt.orig/src/security_selinux.c ./libvirt/src/security_selinux.c --- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500 +++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500 @@ -354,6 +354,7 @@ SELinuxSetFilecon(virConnectPtr conn, co
static int SELinuxRestoreSecurityImageLabel(virConnectPtr conn, + virDomainObjPtr vm, virDomainDiskDefPtr disk) { struct stat buf; @@ -423,7 +429,8 @@ SELinuxRestoreSecurityLabel(virConnectPt int rc = 0; if (secdef->imagelabel) { for (i = 0 ; i < vm->def->ndisks ; i++) { - if (SELinuxRestoreSecurityImageLabel(conn, vm->def->disks[i]) < 0) + if (SELinuxRestoreSecurityImageLabel(conn, vm, + vm->def->disks[i]) < 0) rc = -1; } VIR_FREE(secdef->model);
ACK to this part
diff -Nurp ./libvirt.orig/src/security_selinux.c ./libvirt/src/security_selinux.c --- ./libvirt.orig/src/security_selinux.c 2009-09-02 14:34:08.000000000 -0500 +++ ./libvirt/src/security_selinux.c 2009-09-08 15:32:22.000000000 -0500 @@ -364,6 +365,11 @@ SELinuxRestoreSecurityImageLabel(virConn char *newpath = NULL; const char *path = disk->src;
+ if (&vm->def->seclabel == NULL) { + virSecurityReportError(conn, VIR_ERR_ERROR, _("seclabel is NULL")); + return rc; + } + /* Don't restore labels on readoly/shared disks, because * other VMs may still be accessing these * Alternatively we could iterate over all running
This should have ATTRIBUTE_USED instead Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|