Commit 15021f31 authored by John Johansen's avatar John Johansen Committed by Jiri Slaby

apparmor: ensure the target profile name is always audited

commit f7da2de0 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: default avatarJohn Johansen <john.johansen@canonical.com>
Acked-by: default avatarSeth Arnold <seth.arnold@canonical.com>
Acked-by: default avatarJeff Mahoney <jeffm@suse.com>
Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
parent 26805d78
...@@ -348,7 +348,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -348,7 +348,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
file_inode(bprm->file)->i_uid, file_inode(bprm->file)->i_uid,
file_inode(bprm->file)->i_mode 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); int error = cap_bprm_set_creds(bprm);
if (error) if (error)
return error; return error;
...@@ -403,6 +403,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -403,6 +403,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (cxt->onexec) { if (cxt->onexec) {
struct file_perms cp; struct file_perms cp;
info = "change_profile onexec"; info = "change_profile onexec";
new_profile = aa_get_newest_profile(cxt->onexec);
if (!(perms.allow & AA_MAY_ONEXEC)) if (!(perms.allow & AA_MAY_ONEXEC))
goto audit; goto audit;
...@@ -417,7 +418,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -417,7 +418,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (!(cp.allow & AA_MAY_ONEXEC)) if (!(cp.allow & AA_MAY_ONEXEC))
goto audit; goto audit;
new_profile = aa_get_newest_profile(cxt->onexec);
goto apply; goto apply;
} }
...@@ -449,10 +449,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -449,10 +449,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (!new_profile) { if (!new_profile) {
error = -ENOMEM; error = -ENOMEM;
info = "could not create null profile"; info = "could not create null profile";
} else { } else
error = -EACCES; error = -EACCES;
target = new_profile->base.hname;
}
perms.xindex |= AA_X_UNSAFE; perms.xindex |= AA_X_UNSAFE;
} else } else
/* fail exec */ /* fail exec */
...@@ -463,7 +461,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -463,7 +461,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
* fail the exec. * fail the exec.
*/ */
if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) { if (bprm->unsafe & LSM_UNSAFE_NO_NEW_PRIVS) {
aa_put_profile(new_profile);
error = -EPERM; error = -EPERM;
goto cleanup; goto cleanup;
} }
...@@ -478,10 +475,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -478,10 +475,8 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) {
error = may_change_ptraced_domain(current, new_profile); error = may_change_ptraced_domain(current, new_profile);
if (error) { if (error)
aa_put_profile(new_profile);
goto audit; goto audit;
}
} }
/* Determine if secure exec is needed. /* Determine if secure exec is needed.
...@@ -502,7 +497,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -502,7 +497,6 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
bprm->unsafe |= AA_SECURE_X_NEEDED; bprm->unsafe |= AA_SECURE_X_NEEDED;
} }
apply: apply:
target = new_profile->base.hname;
/* when transitioning profiles clear unsafe personality bits */ /* when transitioning profiles clear unsafe personality bits */
bprm->per_clear |= PER_CLEAR_ON_SETID; bprm->per_clear |= PER_CLEAR_ON_SETID;
...@@ -510,15 +504,19 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) ...@@ -510,15 +504,19 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm)
aa_put_profile(cxt->profile); aa_put_profile(cxt->profile);
/* transfer new profile reference will be released when cxt is freed */ /* transfer new profile reference will be released when cxt is freed */
cxt->profile = new_profile; cxt->profile = new_profile;
new_profile = NULL;
/* clear out all temporary/transitional state from the context */ /* clear out all temporary/transitional state from the context */
aa_clear_task_cxt_trans(cxt); aa_clear_task_cxt_trans(cxt);
audit: audit:
error = aa_audit_file(profile, &perms, GFP_KERNEL, OP_EXEC, MAY_EXEC, 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: cleanup:
aa_put_profile(new_profile);
aa_put_profile(profile); aa_put_profile(profile);
kfree(buffer); kfree(buffer);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment