[libvirt] [PATCH 1/3] Set proper selinux label on image file during qemu domain restore

[Daniel P. Berrange]

On Fri, Jun 25, 2010 at 07:42:13AM -0400, Laine Stump wrote:
> Since vm->def->os.kernel (the normal place the path is found) is null
> during a domain restore, use the stdin_path that is passed into
> SELinuxSetSecurityAllLabel instead.
> 
> Also restore the label to its original value after qemu is finished
> with the file.
> 
> Prior to this patch, qemu domain restore did not function properly if
> selinux was set to enforce.
> ---
>  src/qemu/qemu_driver.c          |    6 +++++-
>  src/security/security_selinux.c |   12 ++++++++----
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 9f4e082..9140b50 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -6208,7 +6208,6 @@ error:
>      return -1;
>  }
>  
> -/* TODO: check seclabel restore */
>  static int ATTRIBUTE_NONNULL(6)
>  qemudDomainSaveImageStartVM(virConnectPtr conn,
>                              struct qemud_driver *driver,
> @@ -6320,6 +6319,11 @@ qemudDomainSaveImageStartVM(virConnectPtr conn,
>      ret = 0;
>  
>  out:
> +    if (driver->securityDriver &&
> +        driver->securityDriver->domainRestoreSavedStateLabel &&
> +        driver->securityDriver->domainRestoreSavedStateLabel(vm, path) == -1)
> +        VIR_WARN("failed to restore save state label on %s", path);
> +
>      return ret;
>  }
>  
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 2b43f2d..7500f1d 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -859,7 +859,7 @@ SELinuxClearSecuritySocketLabel(virSecurityDriverPtr drv,
>  }
>  
>  static int
> -SELinuxSetSecurityAllLabel(virDomainObjPtr vm, const char *stdin_path ATTRIBUTE_UNUSED)
> +SELinuxSetSecurityAllLabel(virDomainObjPtr vm, const char *stdin_path)
>  {
>      const virSecurityLabelDefPtr secdef = &vm->def->seclabel;
>      int i;
> @@ -882,9 +882,13 @@ SELinuxSetSecurityAllLabel(virDomainObjPtr vm, const char *stdin_path ATTRIBUTE_
>              return -1;
>      }
>  
> -    if (vm->def->os.kernel &&
> -        SELinuxSetFilecon(vm->def->os.kernel, default_content_context) < 0)
> -        return -1;
> +    if (vm->def->os.kernel) {
> +        if (SELinuxSetFilecon(vm->def->os.kernel, default_content_context) < 0)
> +            return -1;
> +    } else if (stdin_path) {
> +        if (SELinuxSetFilecon(stdin_path, default_content_context) < 0)
> +            return -1;
> +    }

This doesn't make sense to me. Labelling of the kernel and labeling of
stdin_path are completely separate tasks, so shouldn't be in an if/elseif
arrangement.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London    -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org        -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|