| From 9cc2c57c52af98c903f75165faa0005a10b8d9af Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 12 May 2021 15:33:08 +0200 |
| Subject: ptrace: make ptrace() fail if the tracee changed its pid unexpectedly |
| |
| From: Oleg Nesterov <oleg@redhat.com> |
| |
| [ Upstream commit dbb5afad100a828c97e012c6106566d99f041db6 ] |
| |
| Suppose we have 2 threads, the group-leader L and a sub-theread T, |
| both parked in ptrace_stop(). Debugger tries to resume both threads |
| and does |
| |
| ptrace(PTRACE_CONT, T); |
| ptrace(PTRACE_CONT, L); |
| |
| If the sub-thread T execs in between, the 2nd PTRACE_CONT doesn not |
| resume the old leader L, it resumes the post-exec thread T which was |
| actually now stopped in PTHREAD_EVENT_EXEC. In this case the |
| PTHREAD_EVENT_EXEC event is lost, and the tracer can't know that the |
| tracee changed its pid. |
| |
| This patch makes ptrace() fail in this case until debugger does wait() |
| and consumes PTHREAD_EVENT_EXEC which reports old_pid. This affects all |
| ptrace requests except the "asynchronous" PTRACE_INTERRUPT/KILL. |
| |
| The patch doesn't add the new PTRACE_ option to not complicate the API, |
| and I _hope_ this won't cause any noticeable regression: |
| |
| - If debugger uses PTRACE_O_TRACEEXEC and the thread did an exec |
| and the tracer does a ptrace request without having consumed |
| the exec event, it's 100% sure that the thread the ptracer |
| thinks it is targeting does not exist anymore, or isn't the |
| same as the one it thinks it is targeting. |
| |
| - To some degree this patch adds nothing new. In the scenario |
| above ptrace(L) can fail with -ESRCH if it is called after the |
| execing sub-thread wakes the leader up and before it "steals" |
| the leader's pid. |
| |
| Test-case: |
| |
| #include <stdio.h> |
| #include <unistd.h> |
| #include <signal.h> |
| #include <sys/ptrace.h> |
| #include <sys/wait.h> |
| #include <errno.h> |
| #include <pthread.h> |
| #include <assert.h> |
| |
| void *tf(void *arg) |
| { |
| execve("/usr/bin/true", NULL, NULL); |
| assert(0); |
| |
| return NULL; |
| } |
| |
| int main(void) |
| { |
| int leader = fork(); |
| if (!leader) { |
| kill(getpid(), SIGSTOP); |
| |
| pthread_t th; |
| pthread_create(&th, NULL, tf, NULL); |
| for (;;) |
| pause(); |
| |
| return 0; |
| } |
| |
| waitpid(leader, NULL, WSTOPPED); |
| |
| ptrace(PTRACE_SEIZE, leader, 0, |
| PTRACE_O_TRACECLONE | PTRACE_O_TRACEEXEC); |
| waitpid(leader, NULL, 0); |
| |
| ptrace(PTRACE_CONT, leader, 0,0); |
| waitpid(leader, NULL, 0); |
| |
| int status, thread = waitpid(-1, &status, 0); |
| assert(thread > 0 && thread != leader); |
| assert(status == 0x80137f); |
| |
| ptrace(PTRACE_CONT, thread, 0,0); |
| /* |
| * waitid() because waitpid(leader, &status, WNOWAIT) does not |
| * report status. Why ???? |
| * |
| * Why WEXITED? because we have another kernel problem connected |
| * to mt-exec. |
| */ |
| siginfo_t info; |
| assert(waitid(P_PID, leader, &info, WSTOPPED|WEXITED|WNOWAIT) == 0); |
| assert(info.si_pid == leader && info.si_status == 0x0405); |
| |
| /* OK, it sleeps in ptrace(PTRACE_EVENT_EXEC == 0x04) */ |
| assert(ptrace(PTRACE_CONT, leader, 0,0) == -1); |
| assert(errno == ESRCH); |
| |
| assert(leader == waitpid(leader, &status, WNOHANG)); |
| assert(status == 0x04057f); |
| |
| assert(ptrace(PTRACE_CONT, leader, 0,0) == 0); |
| |
| return 0; |
| } |
| |
| Signed-off-by: Oleg Nesterov <oleg@redhat.com> |
| Reported-by: Simon Marchi <simon.marchi@efficios.com> |
| Acked-by: "Eric W. Biederman" <ebiederm@xmission.com> |
| Acked-by: Pedro Alves <palves@redhat.com> |
| Acked-by: Simon Marchi <simon.marchi@efficios.com> |
| Acked-by: Jan Kratochvil <jan.kratochvil@redhat.com> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| kernel/ptrace.c | 18 +++++++++++++++++- |
| 1 file changed, 17 insertions(+), 1 deletion(-) |
| |
| diff --git a/kernel/ptrace.c b/kernel/ptrace.c |
| index ea3370e205fb..4f10223bc7b0 100644 |
| --- a/kernel/ptrace.c |
| +++ b/kernel/ptrace.c |
| @@ -159,6 +159,21 @@ void __ptrace_unlink(struct task_struct *child) |
| spin_unlock(&child->sighand->siglock); |
| } |
| |
| +static bool looks_like_a_spurious_pid(struct task_struct *task) |
| +{ |
| + if (task->exit_code != ((PTRACE_EVENT_EXEC << 8) | SIGTRAP)) |
| + return false; |
| + |
| + if (task_pid_vnr(task) == task->ptrace_message) |
| + return false; |
| + /* |
| + * The tracee changed its pid but the PTRACE_EVENT_EXEC event |
| + * was not wait()'ed, most probably debugger targets the old |
| + * leader which was destroyed in de_thread(). |
| + */ |
| + return true; |
| +} |
| + |
| /* Ensure that nothing can wake it up, even SIGKILL */ |
| static bool ptrace_freeze_traced(struct task_struct *task) |
| { |
| @@ -169,7 +184,8 @@ static bool ptrace_freeze_traced(struct task_struct *task) |
| return ret; |
| |
| spin_lock_irq(&task->sighand->siglock); |
| - if (task_is_traced(task) && !__fatal_signal_pending(task)) { |
| + if (task_is_traced(task) && !looks_like_a_spurious_pid(task) && |
| + !__fatal_signal_pending(task)) { |
| task->state = __TASK_TRACED; |
| ret = true; |
| } |
| -- |
| 2.30.2 |
| |