aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOleg Nesterov2013-02-19 07:56:52 -0600
committerGreg Kroah-Hartman2013-03-03 16:09:06 -0600
commita214998c48e204227d52a81dd459ea51a8a9ae36 (patch)
tree60adb565e2de5615f507ba707c844308e7dffe79
parent964b12560e1d50f31bc1cc0ac662d52bdbdb6f40 (diff)
downloadkernel-common-a214998c48e204227d52a81dd459ea51a8a9ae36.tar.gz
kernel-common-a214998c48e204227d52a81dd459ea51a8a9ae36.tar.xz
kernel-common-a214998c48e204227d52a81dd459ea51a8a9ae36.zip
ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL
Upstream commit 9899d11f654474d2d54ea52ceaa2a1f4db3abd68. putreg() assumes that the tracee is not running and pt_regs_access() can safely play with its stack. However a killed tracee can return from ptrace_stop() to the low-level asm code and do RESTORE_REST, this means that debugger can actually read/modify the kernel stack until the tracee does SAVE_REST again. set_task_blockstep() can race with SIGKILL too and in some sense this race is even worse, the very fact the tracee can be woken up breaks the logic. As Linus suggested we can clear TASK_WAKEKILL around the arch_ptrace() call, this ensures that nobody can ever wakeup the tracee while the debugger looks at it. Not only this fixes the mentioned problems, we can do some cleanups/simplifications in arch_ptrace() paths. Probably ptrace_unfreeze_traced() needs more callers, for example it makes sense to make the tracee killable for oom-killer before access_process_vm(). While at it, add the comment into may_ptrace_stop() to explain why ptrace_stop() still can't rely on SIGKILL and signal_pending_state(). Reported-by: Salman Qazi <sqazi@google.com> Reported-by: Suleiman Souhlal <suleiman@google.com> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Oleg Nesterov <oleg@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Michal Hocko <mhocko@suse.cz> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--kernel/ptrace.c59
-rw-r--r--kernel/signal.c5
2 files changed, 55 insertions, 9 deletions
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index d77fa9eb082..40581ee5680 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -38,6 +38,36 @@ void __ptrace_link(struct task_struct *child, struct task_struct *new_parent)
38 child->parent = new_parent; 38 child->parent = new_parent;
39} 39}
40 40
41/* Ensure that nothing can wake it up, even SIGKILL */
42static bool ptrace_freeze_traced(struct task_struct *task)
43{
44 bool ret = false;
45
46 spin_lock_irq(&task->sighand->siglock);
47 if (task_is_traced(task) && !__fatal_signal_pending(task)) {
48 task->state = __TASK_TRACED;
49 ret = true;
50 }
51 spin_unlock_irq(&task->sighand->siglock);
52
53 return ret;
54}
55
56static void ptrace_unfreeze_traced(struct task_struct *task)
57{
58 if (task->state != __TASK_TRACED)
59 return;
60
61 WARN_ON(!task->ptrace || task->parent != current);
62
63 spin_lock_irq(&task->sighand->siglock);
64 if (__fatal_signal_pending(task))
65 wake_up_state(task, __TASK_TRACED);
66 else
67 task->state = TASK_TRACED;
68 spin_unlock_irq(&task->sighand->siglock);
69}
70
41/** 71/**
42 * __ptrace_unlink - unlink ptracee and restore its execution state 72 * __ptrace_unlink - unlink ptracee and restore its execution state
43 * @child: ptracee to be unlinked 73 * @child: ptracee to be unlinked
@@ -112,23 +142,29 @@ int ptrace_check_attach(struct task_struct *child, int kill)
112 * be changed by us so it's not changing right after this. 142 * be changed by us so it's not changing right after this.
113 */ 143 */
114 read_lock(&tasklist_lock); 144 read_lock(&tasklist_lock);
115 if ((child->ptrace & PT_PTRACED) && child->parent == current) { 145 if (child->ptrace && child->parent == current) {
146 WARN_ON(child->state == __TASK_TRACED);
116 /* 147 /*
117 * child->sighand can't be NULL, release_task() 148 * child->sighand can't be NULL, release_task()
118 * does ptrace_unlink() before __exit_signal(). 149 * does ptrace_unlink() before __exit_signal().
119 */ 150 */
120 spin_lock_irq(&child->sighand->siglock); 151 if (kill || ptrace_freeze_traced(child))
121 WARN_ON_ONCE(task_is_stopped(child));
122 if (task_is_traced(child) || kill)
123 ret = 0; 152 ret = 0;
124 spin_unlock_irq(&child->sighand->siglock);
125 } 153 }
126 read_unlock(&tasklist_lock); 154 read_unlock(&tasklist_lock);
127 155
128 if (!ret && !kill) 156 if (!ret && !kill) {
129 ret = wait_task_inactive(child, TASK_TRACED) ? 0 : -ESRCH; 157 if (!wait_task_inactive(child, __TASK_TRACED)) {
158 /*
159 * This can only happen if may_ptrace_stop() fails and
160 * ptrace_stop() changes ->state back to TASK_RUNNING,
161 * so we should not worry about leaking __TASK_TRACED.
162 */
163 WARN_ON(child->state == __TASK_TRACED);
164 ret = -ESRCH;
165 }
166 }
130 167
131 /* All systems go.. */
132 return ret; 168 return ret;
133} 169}
134 170
@@ -777,6 +813,8 @@ SYSCALL_DEFINE4(ptrace, long, request, long, pid, unsigned long, addr,
777 goto out_put_task_struct; 813 goto out_put_task_struct;
778 814
779 ret = arch_ptrace(child, request, addr, data); 815 ret = arch_ptrace(child, request, addr, data);
816 if (ret || request != PTRACE_DETACH)
817 ptrace_unfreeze_traced(child);
780 818
781 out_put_task_struct: 819 out_put_task_struct:
782 put_task_struct(child); 820 put_task_struct(child);
@@ -915,8 +953,11 @@ asmlinkage long compat_sys_ptrace(compat_long_t request, compat_long_t pid,
915 } 953 }
916 954
917 ret = ptrace_check_attach(child, request == PTRACE_KILL); 955 ret = ptrace_check_attach(child, request == PTRACE_KILL);
918 if (!ret) 956 if (!ret) {
919 ret = compat_arch_ptrace(child, request, addr, data); 957 ret = compat_arch_ptrace(child, request, addr, data);
958 if (ret || request != PTRACE_DETACH)
959 ptrace_unfreeze_traced(child);
960 }
920 961
921 out_put_task_struct: 962 out_put_task_struct:
922 put_task_struct(child); 963 put_task_struct(child);
diff --git a/kernel/signal.c b/kernel/signal.c
index 8b0dd5bb4da..51f2e694ec6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1669,6 +1669,10 @@ static inline int may_ptrace_stop(void)
1669 * If SIGKILL was already sent before the caller unlocked 1669 * If SIGKILL was already sent before the caller unlocked
1670 * ->siglock we must see ->core_state != NULL. Otherwise it 1670 * ->siglock we must see ->core_state != NULL. Otherwise it
1671 * is safe to enter schedule(). 1671 * is safe to enter schedule().
1672 *
1673 * This is almost outdated, a task with the pending SIGKILL can't
1674 * block in TASK_TRACED. But PTRACE_EVENT_EXIT can be reported
1675 * after SIGKILL was already dequeued.
1672 */ 1676 */
1673 if (unlikely(current->mm->core_state) && 1677 if (unlikely(current->mm->core_state) &&
1674 unlikely(current->mm == current->parent->mm)) 1678 unlikely(current->mm == current->parent->mm))
@@ -1800,6 +1804,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info)
1800 if (gstop_done) 1804 if (gstop_done)
1801 do_notify_parent_cldstop(current, false, why); 1805 do_notify_parent_cldstop(current, false, why);
1802 1806
1807 /* tasklist protects us from ptrace_freeze_traced() */
1803 __set_current_state(TASK_RUNNING); 1808 __set_current_state(TASK_RUNNING);
1804 if (clear_code) 1809 if (clear_code)
1805 current->exit_code = 0; 1810 current->exit_code = 0;