Messages in this thread Patch in this message | | | Date | Sun, 13 Sep 2015 00:56:25 +0200 | From | Willy Tarreau <> | Subject | [PATCH 2.6.32 19/62] ptrace: fix race between ptrace_resume() and wait_task_stopped() |
| |
2.6.32-longterm review patch. If anyone has any objections, please let me know.
------------------
From: Oleg Nesterov <oleg@redhat.com>
commit b72c186999e689cb0b055ab1c7b3cd8fffbeb5ed upstream.
ptrace_resume() is called when the tracee is still __TASK_TRACED. We set tracee->exit_code and then wake_up_state() changes tracee->state. If the tracer's sub-thread does wait() in between, task_stopped_code(ptrace => T) wrongly looks like another report from tracee.
This confuses debugger, and since wait_task_stopped() clears ->exit_code the tracee can miss a signal.
Test-case:
#include <stdio.h> #include <unistd.h> #include <sys/wait.h> #include <sys/ptrace.h> #include <pthread.h> #include <assert.h>
int pid;
void *waiter(void *arg) { int stat;
for (;;) { assert(pid == wait(&stat)); assert(WIFSTOPPED(stat)); if (WSTOPSIG(stat) == SIGHUP) continue;
assert(WSTOPSIG(stat) == SIGCONT); printf("ERR! extra/wrong report:%x\n", stat); } }
int main(void) { pthread_t thread;
pid = fork(); if (!pid) { assert(ptrace(PTRACE_TRACEME, 0,0,0) == 0); for (;;) kill(getpid(), SIGHUP); }
assert(pthread_create(&thread, NULL, waiter, NULL) == 0);
for (;;) ptrace(PTRACE_CONT, pid, 0, SIGCONT);
return 0; }
Note for stable: the bug is very old, but without 9899d11f6544 "ptrace: ensure arch_ptrace/ptrace_request can never race with SIGKILL" the fix should use lock_task_sighand(child).
Signed-off-by: Oleg Nesterov <oleg@redhat.com> Reported-by: Pavel Labath <labath@google.com> Tested-by: Pavel Labath <labath@google.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> Signed-off-by: Ben Hutchings <ben@decadent.org.uk> (cherry picked from commit e3f81ba2f0546f030fc234f7aade3016532c75b1)
Signed-off-by: Willy Tarreau <w@1wt.eu> --- kernel/ptrace.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 4185220..03da336 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -516,6 +516,8 @@ static int ptrace_setsiginfo(struct task_struct *child, const siginfo_t *info) static int ptrace_resume(struct task_struct *child, long request, long data) { + bool need_siglock; + if (!valid_signal(data)) return -EIO; @@ -543,8 +545,26 @@ static int ptrace_resume(struct task_struct *child, long request, long data) user_disable_single_step(child); } + /* + * Change ->exit_code and ->state under siglock to avoid the race + * with wait_task_stopped() in between; a non-zero ->exit_code will + * wrongly look like another report from tracee. + * + * Note that we need siglock even if ->exit_code == data and/or this + * status was not reported yet, the new status must not be cleared by + * wait_task_stopped() after resume. + * + * If data == 0 we do not care if wait_task_stopped() reports the old + * status and clears the code too; this can't race with the tracee, it + * takes siglock after resume. + */ + need_siglock = data && !thread_group_empty(current); + if (need_siglock) + spin_lock_irq(&child->sighand->siglock); child->exit_code = data; wake_up_state(child, __TASK_TRACED); + if (need_siglock) + spin_unlock_irq(&child->sighand->siglock); return 0; } -- 1.7.12.2.21.g234cd45.dirty
| |