From dd0c6e86f66080869ca0a48c78fb9bfbe4cf156f Mon Sep 17 00:00:00 2001 From: John Johansen Date: Tue, 8 Oct 2013 05:37:18 -0700 Subject: apparmor: fix capability to not use the current task, during reporting Mediation is based off of the cred but auditing includes the current task which may not be related to the actual request. Signed-off-by: John Johansen --- security/apparmor/domain.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/apparmor/domain.c') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index 26c607c971f5..e5538a12d162 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -75,7 +75,7 @@ static int may_change_ptraced_domain(struct task_struct *task, if (!tracer || unconfined(tracerp)) goto out; - error = aa_may_ptrace(tracer, tracerp, to_profile, PTRACE_MODE_ATTACH); + error = aa_may_ptrace(tracerp, to_profile, PTRACE_MODE_ATTACH); out: rcu_read_unlock(); -- cgit v1.2.3 From 51775fe736f053329faf0f5de7c679ee4cb0023d Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Tue, 8 Oct 2013 05:46:03 -0700 Subject: apparmor: remove the "task" arg from may_change_ptraced_domain() Unless task == current ptrace_parent(task) is not safe even under rcu_read_lock() and most of the current users are not right. So may_change_ptraced_domain(task) looks wrong as well. However it is always called with task == current so the code is actually fine. Remove this argument to make this fact clear. Note: perhaps we should simply kill ptrace_parent(), it buys almost nothing. And it is obviously racy, perhaps this should be fixed. Signed-off-by: Oleg Nesterov Signed-off-by: John Johansen --- security/apparmor/domain.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'security/apparmor/domain.c') diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c index e5538a12d162..452567d3a08e 100644 --- a/security/apparmor/domain.c +++ b/security/apparmor/domain.c @@ -50,23 +50,21 @@ void aa_free_domain_entries(struct aa_domain *domain) /** * may_change_ptraced_domain - check if can change profile on ptraced task - * @task: task we want to change profile of (NOT NULL) * @to_profile: profile to change to (NOT NULL) * - * Check if the task is ptraced and if so if the tracing task is allowed + * Check if current is ptraced and if so if the tracing task is allowed * to trace the new domain * * Returns: %0 or error if change not allowed */ -static int may_change_ptraced_domain(struct task_struct *task, - struct aa_profile *to_profile) +static int may_change_ptraced_domain(struct aa_profile *to_profile) { struct task_struct *tracer; struct aa_profile *tracerp = NULL; int error = 0; rcu_read_lock(); - tracer = ptrace_parent(task); + tracer = ptrace_parent(current); if (tracer) /* released below */ tracerp = aa_get_task_profile(tracer); @@ -477,7 +475,7 @@ 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); + error = may_change_ptraced_domain(new_profile); if (error) { aa_put_profile(new_profile); goto audit; @@ -690,7 +688,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) } } - error = may_change_ptraced_domain(current, hat); + error = may_change_ptraced_domain(hat); if (error) { info = "ptraced"; error = -EPERM; @@ -829,7 +827,7 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec, } /* check if tracing task is allowed to trace target domain */ - error = may_change_ptraced_domain(current, target); + error = may_change_ptraced_domain(target); if (error) { info = "ptrace prevents transition"; goto audit; -- cgit v1.2.3