[apparmor] [PATCH 10/16] apparmor: fix: log failures for all profiles in a set

[Seth Arnold]

On Wed, Apr 20, 2016 at 11:52:52PM -0700, John Johansen wrote:
> currently only the profile that is causing the failure is logged. This
> makes it more confusing than necessary about which profiles loaded
> and which didn't. So make sure to log success and failure messages for
> all profiles in the set being loaded.
> 
> Signed-off-by: John Johansen <john.johansen at canonical.com>

Very neat.

Acked-by: Seth Arnold <seth.arnold at canonical.com>

Thanks

> ---
>  security/apparmor/policy.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/security/apparmor/policy.c b/security/apparmor/policy.c
> index 455c9f8..db31bc5 100644
> --- a/security/apparmor/policy.c
> +++ b/security/apparmor/policy.c
> @@ -1067,7 +1067,7 @@ static int __lookup_replace(struct aa_namespace *ns, const char *hname,
>   */
>  ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
>  {
> -	const char *ns_name, *name = NULL, *info = NULL;
> +	const char *ns_name, *info = NULL;
>  	struct aa_namespace *ns = NULL;
>  	struct aa_load_ent *ent, *tmp;
>  	int op = OP_PROF_REPL;
> @@ -1082,18 +1082,15 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
>  	/* released below */
>  	ns = aa_prepare_namespace(ns_name);
>  	if (!ns) {
> -		info = "failed to prepare namespace";
> -		error = -ENOMEM;
> -		name = ns_name;
> -		goto fail;
> +		error = audit_policy(op, GFP_KERNEL, ns_name,
> +				     "failed to prepare namespace", -ENOMEM);
> +		goto free;
>  	}
>  
>  	mutex_lock(&ns->lock);
>  	/* setup parent and ns info */
>  	list_for_each_entry(ent, &lh, list) {
>  		struct aa_policy *policy;
> -
> -		name = ent->new->base.hname;
>  		error = __lookup_replace(ns, ent->new->base.hname, noreplace,
>  					 &ent->old, &info);
>  		if (error)
> @@ -1121,7 +1118,6 @@ ssize_t aa_replace_profiles(void *udata, size_t size, bool noreplace)
>  			if (!p) {
>  				error = -ENOENT;
>  				info = "parent does not exist";
> -				name = ent->new->base.hname;
>  				goto fail_lock;
>  			}
>  			rcu_assign_pointer(ent->new->parent, aa_get_profile(p));
> @@ -1214,9 +1210,22 @@ out:
>  
>  fail_lock:
>  	mutex_unlock(&ns->lock);
> -fail:
> -	error = audit_policy(op, GFP_KERNEL, name, info, error);
>  
> +	/* audit cause of failure */
> +	op = (!ent->old) ? OP_PROF_LOAD : OP_PROF_REPL;
> +	audit_policy(op, GFP_KERNEL, ent->new->base.hname, info, error);
> +	/* audit status that rest of profiles in the atomic set failed too */
> +	info = "valid profile in failed atomic policy load";
> +	list_for_each_entry(tmp, &lh, list) {
> +		if (tmp == ent) {
> +			info = "unchecked profile in failed atomic policy load";
> +			/* skip entry that caused failure */
> +			continue;
> +		}
> +		op = (!ent->old) ? OP_PROF_LOAD : OP_PROF_REPL;
> +		audit_policy(op, GFP_KERNEL, tmp->new->base.hname, info, error);
> +	}
> +free:
>  	list_for_each_entry_safe(ent, tmp, &lh, list) {
>  		list_del_init(&ent->list);
>  		aa_load_ent_free(ent);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: Digital signature
URL: <https://lists.ubuntu.com/archives/apparmor/attachments/20160427/3d0811f3/attachment.pgp>