diff options
author | John Johansen <john.johansen@canonical.com> | 2016-04-20 14:18:18 -0700 |
---|---|---|
committer | John Johansen <john.johansen@canonical.com> | 2016-07-12 08:43:10 -0700 |
commit | f7da2de01127b58d93cebeab165136d0998e7b1a (patch) | |
tree | e61ea1a2516b02cd42c3275dd29b3d32a93889dd /security | |
parent | 7ee6da25dcce27b6023a8673fdf8be98dcf7cacf (diff) | |
download | op-kernel-dev-f7da2de01127b58d93cebeab165136d0998e7b1a.zip op-kernel-dev-f7da2de01127b58d93cebeab165136d0998e7b1a.tar.gz |
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>
Diffstat (limited to 'security')
-rw-r--r-- | security/apparmor/domain.c | 20 |
1 files changed, 9 insertions, 11 deletions
diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 67a7418..fc3036b 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); |