[libvirt] [PATCH v2] security_selinux: Fix crash in virSecuritySELinuxRestoreFileLabel

[John Ferlan]

On 01/21/2016 07:52 AM, Shanzhi Yu wrote:
> virSecuritySELinuxRestoreFileLabel should never be called with NULL path
> add check before call this function in case of causeing libvirtd crash
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1300532
> Signed-off-by: Shanzhi Yu <shyu at redhat.com>
> ---
>  src/security/security_selinux.c | 25 +++++++++++++++++--------
>  1 file changed, 17 insertions(+), 8 deletions(-)
> 

Resolved by:

commit 1794a0103ae4fa91d9c11617e7981471173e27ce
Author: Martin Kletzander <mkletzan at redhat.com>
Date:   Tue Feb 2 22:08:59 2016 +0100

    qemu: Don't crash when create fails early

...

See:

http://www.redhat.com/archives/libvir-list/2016-February/msg00137.html

> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index 9e98635..77e55a3 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1098,7 +1098,8 @@ virSecuritySELinuxRestoreInputLabel(virSecurityManagerPtr mgr,
>  
>      switch ((virDomainInputType) input->type) {
>      case VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH:
> -        rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev);
> +        if (input->source.evdev)
> +            rc = virSecuritySELinuxRestoreFileLabel(mgr, input->source.evdev);
>          break;
>  
>      case VIR_DOMAIN_INPUT_TYPE_MOUSE:
> @@ -1171,7 +1172,9 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr,
>      switch (tpm->type) {
>      case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH:
>          tpmdev = tpm->data.passthrough.source.data.file.path;
> -        rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev);
> +
> +        if (tpmdev)
> +            rc = virSecuritySELinuxRestoreFileLabel(mgr, tpmdev);
>  
>          if ((cancel_path = virTPMCreateCancelPath(tpmdev)) != NULL) {
>              if (virSecuritySELinuxRestoreFileLabel(mgr, cancel_path) < 0)
> @@ -1722,7 +1725,9 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr,
>              if (VIR_STRDUP(path, dev->source.caps.u.storage.block) < 0)
>                  return -1;
>          }
> -        ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
> +        if (path)
> +            ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
> +
>          VIR_FREE(path);
>          break;
>      }
> @@ -1736,7 +1741,8 @@ virSecuritySELinuxRestoreHostdevCapsLabel(virSecurityManagerPtr mgr,
>              if (VIR_STRDUP(path, dev->source.caps.u.misc.chardev) < 0)
>                  return -1;
>          }
> -        ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
> +        if (path)
> +            ret = virSecuritySELinuxRestoreFileLabel(mgr, path);
>          VIR_FREE(path);
>          break;
>      }
> @@ -1876,13 +1882,15 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>      switch (dev_source->type) {
>      case VIR_DOMAIN_CHR_TYPE_DEV:
>      case VIR_DOMAIN_CHR_TYPE_FILE:
> -        if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0)
> -            goto done;
> +        if (dev_source->data.file.path) {
> +            if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0)
> +                goto done;
> +        }
>          ret = 0;
>          break;
>  
>      case VIR_DOMAIN_CHR_TYPE_UNIX:
> -        if (!dev_source->data.nix.listen) {
> +        if (!dev_source->data.nix.listen && dev_source->data.file.path) {
>              if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0)
>                  goto done;
>          }
> @@ -1898,7 +1906,8 @@ virSecuritySELinuxRestoreChardevLabel(virSecurityManagerPtr mgr,
>                  (virSecuritySELinuxRestoreFileLabel(mgr, in) < 0)) {
>                  goto done;
>              }
> -        } else if (virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) {
> +        } else if (dev_source->data.file.path &&
> +                   virSecuritySELinuxRestoreFileLabel(mgr, dev_source->data.file.path) < 0) {
>              goto done;
>          }
>          ret = 0;
>