apparmor: ensure the target profile name is always audited

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>

1 files changed, 9 insertions, 11 deletions

diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
index 67a7418937a5..fc3036b34e51 100644
--- a/security/apparmor/domain.c
+++ b/security/apparmor/domain.c
@@ -346,7 +346,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 = 0;
 
         if (bprm->cred_prepared)
@@ -399,6 +399,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;
 
@@ -413,7 +414,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;
         }
 
@@ -445,10 +445,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 */
@@ -459,7 +457,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;
         }
@@ -474,10 +471,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(new_profile);
-                if (error) {
-                        aa_put_profile(new_profile);
+                if (error)
                         goto audit;
-                }
         }
 
         /* Determine if secure exec is needed.
@@ -498,7 +493,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;
 
@@ -506,15 +500,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);