[libvirt] [PATCH 2/2] apparmor: don't fail on non-apparmor <seclabel>

[Guido Günther]

On Fri, Feb 03, 2017 at 10:32:12AM -0700, Jim Fehlig wrote:
> If the apparmor security driver is loaded/enabled and domain config
> contains a <seclabel> element whose type attribute is not 'apparmor',
> starting the domain fails when attempting to label resources such
> as tap FDs.
> 
> Many of the apparmor driver entry points attempt to retrieve the
> apparmor security label from the domain def, returning failure if
> not found. Functions such as AppArmorSetFDLabel fail even though
> domain config contains an explicit 'none' secuirty driver, e.g.
> 
>   <seclabel type='none' model='none'/>
> 
> Change the entry points to succeed if the domain config <seclabel>
> is not apparmor. This matches the behavior of the selinux driver.
> 
> Signed-off-by: Jim Fehlig <jfehlig at suse.com>
> ---
>  src/security/security_apparmor.c | 58 ++++++++++++----------------------------
>  1 file changed, 17 insertions(+), 41 deletions(-)
> 
> diff --git a/src/security/security_apparmor.c b/src/security/security_apparmor.c
> index ad50b08..f871e60 100644
> --- a/src/security/security_apparmor.c
> +++ b/src/security/security_apparmor.c
> @@ -289,10 +289,7 @@ reload_profile(virSecurityManagerPtr mgr,
>      virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(
>                                                  def, SECURITY_APPARMOR_NAME);
>  
> -    if (!secdef)
> -        return rc;
> -
> -    if (!secdef->relabel)
> +    if (!secdef || !secdef->relabel)
>          return 0;
>  
>      if ((profile_name = get_profile_name(def)) == NULL)
> @@ -435,7 +432,7 @@ AppArmorGenSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>                                                  SECURITY_APPARMOR_NAME);
>  
>      if (!secdef)
> -        return -1;
> +        return 0;
>  
>      if ((secdef->type == VIR_DOMAIN_SECLABEL_STATIC) ||
>          (secdef->type == VIR_DOMAIN_SECLABEL_NONE))
> @@ -495,10 +492,7 @@ AppArmorSetSecurityAllLabel(virSecurityManagerPtr mgr,
>  {
>      virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
>                                                      SECURITY_APPARMOR_NAME);
> -    if (!secdef)
> -        return -1;
> -
> -    if (!secdef->relabel)
> +    if (!secdef || !secdef->relabel)
>          return 0;
>  
>      /* Reload the profile if stdin_path is specified. Note that
> @@ -559,12 +553,11 @@ AppArmorReleaseSecurityLabel(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED,
>  {
>      virSecurityLabelDefPtr secdef = virDomainDefGetSecurityLabelDef(def,
>                                                          SECURITY_APPARMOR_NAME);
> -    if (!secdef)
> -        return -1;
> -
> -    VIR_FREE(secdef->model);
> -    VIR_FREE(secdef->label);
> -    VIR_FREE(secdef->imagelabel);
> +    if (secdef) {
> +        VIR_FREE(secdef->model);
> +        VIR_FREE(secdef->label);
> +        VIR_FREE(secdef->imagelabel);\

The trailing slash can be dropped here. The rest of th code looks good
to me.
 -- Guido