Re: [libvirt] Resubmission: [PATCH 1/6] sVirt AppArmor security driver

[Jamie Strandboge <jamie@xxxxxxxxxxxxx>]

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.
> > 

-- 
Jamie Strandboge             | http://www.canonical.com

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);

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

Attachment: signature.asc
Description: Digital signature

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