diff options
author | Oleg Nesterov <oleg@redhat.com> | 2013-10-08 08:46:03 -0400 |
---|---|---|
committer | John Johansen <john.johansen@canonical.com> | 2013-10-30 00:34:18 -0400 |
commit | 51775fe736f053329faf0f5de7c679ee4cb0023d (patch) | |
tree | 03c7edaa4b4e6b3d78528769202661ce4861a832 | |
parent | 4a7fc3018f05f4305723b508b12f3be13b7c4875 (diff) |
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 <oleg@redhat.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
-rw-r--r-- | security/apparmor/domain.c | 14 |
1 files changed, 6 insertions, 8 deletions
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) | |||
50 | 50 | ||
51 | /** | 51 | /** |
52 | * may_change_ptraced_domain - check if can change profile on ptraced task | 52 | * may_change_ptraced_domain - check if can change profile on ptraced task |
53 | * @task: task we want to change profile of (NOT NULL) | ||
54 | * @to_profile: profile to change to (NOT NULL) | 53 | * @to_profile: profile to change to (NOT NULL) |
55 | * | 54 | * |
56 | * Check if the task is ptraced and if so if the tracing task is allowed | 55 | * Check if current is ptraced and if so if the tracing task is allowed |
57 | * to trace the new domain | 56 | * to trace the new domain |
58 | * | 57 | * |
59 | * Returns: %0 or error if change not allowed | 58 | * Returns: %0 or error if change not allowed |
60 | */ | 59 | */ |
61 | static int may_change_ptraced_domain(struct task_struct *task, | 60 | static int may_change_ptraced_domain(struct aa_profile *to_profile) |
62 | struct aa_profile *to_profile) | ||
63 | { | 61 | { |
64 | struct task_struct *tracer; | 62 | struct task_struct *tracer; |
65 | struct aa_profile *tracerp = NULL; | 63 | struct aa_profile *tracerp = NULL; |
66 | int error = 0; | 64 | int error = 0; |
67 | 65 | ||
68 | rcu_read_lock(); | 66 | rcu_read_lock(); |
69 | tracer = ptrace_parent(task); | 67 | tracer = ptrace_parent(current); |
70 | if (tracer) | 68 | if (tracer) |
71 | /* released below */ | 69 | /* released below */ |
72 | tracerp = aa_get_task_profile(tracer); | 70 | tracerp = aa_get_task_profile(tracer); |
@@ -477,7 +475,7 @@ int apparmor_bprm_set_creds(struct linux_binprm *bprm) | |||
477 | } | 475 | } |
478 | 476 | ||
479 | if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { | 477 | if (bprm->unsafe & (LSM_UNSAFE_PTRACE | LSM_UNSAFE_PTRACE_CAP)) { |
480 | error = may_change_ptraced_domain(current, new_profile); | 478 | error = may_change_ptraced_domain(new_profile); |
481 | if (error) { | 479 | if (error) { |
482 | aa_put_profile(new_profile); | 480 | aa_put_profile(new_profile); |
483 | goto audit; | 481 | goto audit; |
@@ -690,7 +688,7 @@ int aa_change_hat(const char *hats[], int count, u64 token, bool permtest) | |||
690 | } | 688 | } |
691 | } | 689 | } |
692 | 690 | ||
693 | error = may_change_ptraced_domain(current, hat); | 691 | error = may_change_ptraced_domain(hat); |
694 | if (error) { | 692 | if (error) { |
695 | info = "ptraced"; | 693 | info = "ptraced"; |
696 | error = -EPERM; | 694 | error = -EPERM; |
@@ -829,7 +827,7 @@ int aa_change_profile(const char *ns_name, const char *hname, bool onexec, | |||
829 | } | 827 | } |
830 | 828 | ||
831 | /* check if tracing task is allowed to trace target domain */ | 829 | /* check if tracing task is allowed to trace target domain */ |
832 | error = may_change_ptraced_domain(current, target); | 830 | error = may_change_ptraced_domain(target); |
833 | if (error) { | 831 | if (error) { |
834 | info = "ptrace prevents transition"; | 832 | info = "ptrace prevents transition"; |
835 | goto audit; | 833 | goto audit; |