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 /security | |
| 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>
Diffstat (limited to 'security')
| -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; |
