[libvirt] [PATCH v2 7/10] selinux: Resolve resource leak using the default disk label

[Marcelo Cerri]

The fix seems correct to me.

On Fri, Jan 18, 2013 at 09:34:13AM -0500, John Ferlan wrote:
> Commit id a994ef2d1 changed the mechanism to store/update the default
> security label from using disk->seclabels[0] to allocating one on the
> fly. That change allocated the label, but never saved it.  This patch
> will save the label. The new virDomainDiskDefAddSecurityLabelDef() is
> a copy of the virDomainDefAddSecurityLabelDef().
> ---
>  src/conf/domain_conf.c          | 51 ++++++++++++++++++++++++++++++-----------
>  src/conf/domain_conf.h          |  3 +++
>  src/security/security_selinux.c |  6 ++---
>  3 files changed, 44 insertions(+), 16 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 0b9ba13..7640af7 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -16041,26 +16041,51 @@ virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model)
>  {
>      virSecurityLabelDefPtr seclabel = NULL;
> 
> -    if (VIR_ALLOC(seclabel) < 0) {
> -        virReportOOMError();
> -        return NULL;
> -    }
> +    if (VIR_ALLOC(seclabel) < 0)
> +        goto no_memory;
> 
>      if (model) {
>          seclabel->model = strdup(model);
> -        if (seclabel->model == NULL) {
> -            virReportOOMError();
> -            virSecurityLabelDefFree(seclabel);
> -            return NULL;
> -        }
> +        if (seclabel->model == NULL)
> +            goto no_memory;
>      }
> 
> -    if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0) {
> -        virReportOOMError();
> -        virSecurityLabelDefFree(seclabel);
> -        return NULL;
> +    if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
> +        goto no_memory;
> +
> +    def->seclabels[def->nseclabels - 1] = seclabel;
> +
> +    return seclabel;
> +
> +no_memory:
> +    virReportOOMError();
> +    virSecurityLabelDefFree(seclabel);
> +    return NULL;
> +}
> +
> +virSecurityDeviceLabelDefPtr
> +virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model)
> +{
> +    virSecurityDeviceLabelDefPtr seclabel = NULL;
> +
> +    if (VIR_ALLOC(seclabel) < 0)
> +        goto no_memory;
> +
> +    if (model) {
> +        seclabel->model = strdup(model);
> +        if (seclabel->model == NULL)
> +            goto no_memory;
>      }
> +
> +    if (VIR_EXPAND_N(def->seclabels, def->nseclabels, 1) < 0)
> +        goto no_memory;
> +
>      def->seclabels[def->nseclabels - 1] = seclabel;
> 
>      return seclabel;
> +
> +no_memory:
> +    virReportOOMError();
> +    virSecurityDeviceLabelDefFree(seclabel);
> +    return NULL;
>  }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ce36003..9770ffb 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2222,6 +2222,9 @@ virDomainChrDefGetSecurityLabelDef(virDomainChrDefPtr def, const char *model);
>  virSecurityLabelDefPtr
>  virDomainDefAddSecurityLabelDef(virDomainDefPtr def, const char *model);
> 
> +virSecurityDeviceLabelDefPtr
> +virDomainDiskDefAddSecurityLabelDef(virDomainDiskDefPtr def, const char *model);
> +
>  typedef const char* (*virEventActionToStringFunc)(int type);
>  typedef int (*virEventActionFromStringFunc)(const char *type);
> 
> diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
> index b5e1a9a..511e923 100644
> --- a/src/security/security_selinux.c
> +++ b/src/security/security_selinux.c
> @@ -1095,10 +1095,10 @@ virSecuritySELinuxSetSecurityFileLabel(virDomainDiskDefPtr disk,
>      if (ret == 1 && !disk_seclabel) {
>          /* If we failed to set a label, but virt_use_nfs let us
>           * proceed anyway, then we don't need to relabel later.  */
> -        if (VIR_ALLOC(disk_seclabel) < 0) {
> -            virReportOOMError();
> +        disk_seclabel =
> +            virDomainDiskDefAddSecurityLabelDef(disk, SECURITY_SELINUX_NAME);
> +        if (!disk_seclabel)
>              return -1;
> -        }
>          disk_seclabel->norelabel = true;
>          ret = 0;
>      }
> -- 
> 1.7.11.7
> 
> --
> libvir-list mailing list
> libvir-list at redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
>