[PATCH 02/11] AppArmor: Fix refcount bug when exec fails

[Andy Whitcroft]

On Tue, Apr 13, 2010 at 12:09:31AM -0700, john.johansen at canonical.com wrote:
> From: John Johansen <john.johansen at canonical.com>
> 
> OriginalAuthor: John Johansen <john.johansen at canonical.com>
> OriginalLocation: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparm$
> commit: af11e86b3b91f0aaf5155e73d0d3f196124b25e2
> BugLink: http://bugs.launchpad.net/bugs/562063
> 
> The error case for ptrace permission on exec missed putting the new_profile,
> fix this ommission and consolidate the error cases to a single point.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>
> ---
>  security/apparmor/domain.c |   12 +++++++-----
>  1 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index cd8ec99..2721fcb 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -424,10 +424,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>  	if (!new_profile)
>  		goto audit;
>  
> -	if (profile == new_profile) {
> -		aa_put_profile(new_profile);
> -		goto audit;
> -	}
> +	if (profile == new_profile)
> +		goto abort;
>  
>  	if (bprm->unsafe & LSM_UNSAFE_SHARE) {
>  		/* FIXME: currently don't mediate shared state */
> @@ -438,7 +436,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
>  		sa.base.error = aa_may_change_ptraced_domain(current,
>  							     new_profile);
>  		if (sa.base.error)
> -			goto audit;
> +			goto abort;
>  	}
>  
>  	/* Determine if secure exec is needed.
> @@ -485,6 +483,10 @@ cleanup:
>  	kfree(buffer);
>  
>  	return sa.base.error;
> +
> +abort:
> +	aa_put_profile(new_profile);
> +	goto audit;
>  }

This is a little ugly, but I can see why you want to consolidate them.

>  
>  /**

This routine is generally vile and jumptastic already, so I guess its
not making it any worse.  For a patch this late I would prefer something
vile to a big cleanup.  On balance:

Acked-by: Andy Whitcroft <apw at canonical.com>

-apw