apparmor: ensure the target profile name is always audited

commit f7da2de01127b58d93cebeab165136d0998e7b1a upstream.

The target profile name was not being correctly audited in a few
cases because the target variable was not being set and gotos
passed the code to set it at apply:

Since it is always based on new_profile just drop the target var
and conditionally report based on new_profile.

Signed-off-by: John Johansen <john.johansen@canonical.com>
Acked-by: Seth Arnold <seth.arnold@canonical.com>
Acked-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>

1 files changed, 9 insertions, 11 deletions

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index a59766fe3b7a..1c7763766135 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -348,7 +348,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		file_inode(bprm->file)->i_uid,
 		file_inode(bprm->file)->i_mode
 	};
-	const char *name = NULL, *target = NULL, *info = NULL;
+	const char *name = NULL, *info = NULL;
 	int error = cap_bprm_set_creds(bprm);
 	if (error)
 		return error;
@@ -403,6 +403,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	if (cxt->onexec) {
 		struct file_perms cp;
 		info = "change_profile onexec";
+		new_profile = aa_get_newest_profile(cxt->onexec);
 		if (!(perms.allow & AA_MAY_ONEXEC))
 			goto audit;
 
@@ -417,7 +418,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 
 		if (!(cp.allow & AA_MAY_ONEXEC))
 			goto audit;
-		new_profile = aa_get_newest_profile(cxt->onexec);
 		goto apply;
 	}
 
@@ -449,10 +449,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		if (!new_profile) {
 			error = -ENOMEM;
 			info = "could not create null profile";
-		} else {
+		} else
 			error = -EACCES;
-			target = new_profile->base.hname;
-		}
 		perms.xindex |= AA_X_UNSAFE;
 	} else
 		/* fail exec */
@@ -463,7 +461,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 	 * fail the exec.
 	 */
 	if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) {
-		aa_put_profile(new_profile);
 		error = -EPERM;
 		goto cleanup;
 	}
@@ -478,10 +475,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 
 	if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
 		error = may_change_ptraced_domain(current, new_profile);
-		if (error) {
-			aa_put_profile(new_profile);
+		if (error)
 			goto audit;
-		}
 	}
 
 	/* Determine if secure exec is needed.
@@ -502,7 +497,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
 		bprm->unsafe |= AA_SECURE_X_NEEDED;
 	}
 apply:
-	target = new_profile->base.hname;
 	/* when transitioning profiles clear unsafe personality bits */
 	bprm->per_clear |= PER_CLEAR_ON_SETID;
 
@@ -510,15 +504,19 @@ x_clear:
 	aa_put_profile(cxt->profile);
 	/* transfer new profile reference will be released when cxt is freed */
 	cxt->profile = new_profile;
+	new_profile = NULL;
 
 	/* clear out all temporary/transitional state from the context */
 	aa_clear_task_cxt_trans(cxt);
 
 audit:
 	error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC,
-			      name, target, cond.uid, info, error);
+			      name,
+			      new_profile ? new_profile->base.hname : NULL,
+			      cond.uid, info, error);
 
 cleanup:
+	aa_put_profile(new_profile);
 	aa_put_profile(profile);
 	kfree(buffer);