On Thu, Nov 19, 2009 at 12:11:24PM +0000, Daniel P. Berrange wrote:
> Even though QEMU does not directly open the saved image when
> restoring, it must be correctly labelled to allow QEMU to
> read from it because labelling is passed around with open
> file descriptors.
>
> The labelling should not allow writing to the saved image
> again, only reading.
>
> * src/qemu/qemu_driver.c: Label the save image when restoring
> * src/security/security_driver.h: Add a virSecurityDomainSetSavedStateLabelRO
> method for labelling a saved image for restore
> * src/security/security_selinux.c: Implement labelling of RO
> save images for restore
> ---
> src/qemu/qemu_driver.c | 31 ++++++++++++++++++++-----------
> src/security/security_driver.h | 5 +++++
> src/security/security_selinux.c | 11 +++++++++++
> 3 files changed, 36 insertions(+), 11 deletions(-)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index a4a87ac..d500e14 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -3484,7 +3484,7 @@ static int qemudDomainSave(virDomainPtr dom,
>
> if (driver->securityDriver &&
> driver->securityDriver->domainRestoreSavedStateLabel &&
> - driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, path) == -1)
> + driver->securityDriver->domainRestoreSavedStateLabel(dom->conn, vm, path) == -1)
> goto endjob;
>
> ret = 0;
> @@ -4036,6 +4036,11 @@ static int qemudDomainRestore(virConnectPtr conn,
> if (qemuDomainObjBeginJobWithDriver(driver, vm) < 0)
> goto cleanup;
>
> + if (driver->securityDriver &&
> + driver->securityDriver->domainSetSavedStateLabelRO &&
> + driver->securityDriver->domainSetSavedStateLabelRO(conn, vm, path) == -1)
> + goto endjob;
> +
> if (header.version == 2) {
> const char *intermediate_argv[3] = { NULL, "-dc", NULL };
> const char *prog = qemudSaveCompressionTypeToString(header.compressed);
> @@ -4070,15 +4075,8 @@ static int qemudDomainRestore(virConnectPtr conn,
> close(intermediatefd);
> close(fd);
> fd = -1;
> - if (ret < 0) {
> - if (!vm->persistent) {
> - qemuDomainObjEndJob(vm);
> - virDomainRemoveInactive(&driver->domains,
> - vm);
> - vm = NULL;
> - }
> + if (ret < 0)
> goto endjob;
> - }
>
> event = virDomainEventNewFromObj(vm,
> VIR_DOMAIN_EVENT_STARTED,
> @@ -4102,8 +4100,19 @@ static int qemudDomainRestore(virConnectPtr conn,
> ret = 0;
>
> endjob:
> - if (vm)
> - qemuDomainObjEndJob(vm);
> + qemuDomainObjEndJob(vm);
> + if (driver->securityDriver &&
> + driver->securityDriver->domainRestoreSavedStateLabel &&
> + driver->securityDriver->domainRestoreSavedStateLabel(conn, vm, path) == -1)
> + VIR_WARN("Unable to restore labelling on %s", path);
> +
> + if (ret < 0) {
> + if (!vm->persistent) {
> + virDomainRemoveInactive(&driver->domains,
> + vm);
> + vm = NULL;
> + }
> + }
>
> cleanup:
> virDomainDefFree(def);
> diff --git a/src/security/security_driver.h b/src/security/security_driver.h
> index 5514962..5144976 100644
> --- a/src/security/security_driver.h
> +++ b/src/security/security_driver.h
> @@ -45,7 +45,11 @@ typedef int (*virSecurityDomainSetHostdevLabel) (virConnectPtr conn,
> typedef int (*virSecurityDomainSetSavedStateLabel) (virConnectPtr conn,
> virDomainObjPtr vm,
> const char *savefile);
> +typedef int (*virSecurityDomainSetSavedStateLabelRO) (virConnectPtr conn,
> + virDomainObjPtr vm,
> + const char *savefile);
> typedef int (*virSecurityDomainRestoreSavedStateLabel) (virConnectPtr conn,
> + virDomainObjPtr vm,
> const char *savefile);
> typedef int (*virSecurityDomainGenLabel) (virConnectPtr conn,
> virDomainObjPtr sec);
> @@ -77,6 +81,7 @@ struct _virSecurityDriver {
> virSecurityDomainRestoreHostdevLabel domainRestoreSecurityHostdevLabel;
> virSecurityDomainSetHostdevLabel domainSetSecurityHostdevLabel;
> virSecurityDomainSetSavedStateLabel domainSetSavedStateLabel;
> + virSecurityDomainSetSavedStateLabelRO domainSetSavedStateLabelRO;
> virSecurityDomainRestoreSavedStateLabel domainRestoreSavedStateLabel;
>
> /*
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index bd838e6..49f6746 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -637,7 +637,17 @@ SELinuxSetSavedStateLabel(virConnectPtr conn,
>
>
> static int
> +SELinuxSetSavedStateLabelRO(virConnectPtr conn,
> + virDomainObjPtr vm ATTRIBUTE_UNUSED,
> + const char *savefile)
> +{
> + return SELinuxSetFilecon(conn, savefile, default_content_context);
> +}
> +
> +
> +static int
> SELinuxRestoreSavedStateLabel(virConnectPtr conn,
> + virDomainObjPtr vm ATTRIBUTE_UNUSED,
> const char *savefile)
> {
> return SELinuxRestoreSecurityFileLabel(conn, savefile);
ACK
> @@ -714,5 +724,6 @@ virSecurityDriver virSELinuxSecurityDriver = {
> .domainSetSecurityHostdevLabel = SELinuxSetSecurityHostdevLabel,
> .domainRestoreSecurityHostdevLabel = SELinuxRestoreSecurityHostdevLabel,
> .domainSetSavedStateLabel = SELinuxSetSavedStateLabel,
> + .domainSetSavedStateLabelRO = SELinuxSetSavedStateLabelRO,
> .domainRestoreSavedStateLabel = SELinuxRestoreSavedStateLabel,
> };
maybe one could take the opportunity to convert to the explicit
complete initialization for the security driver too, as we did for
hypervisor ones.
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list