[PATCH] signal/ptrace: Fix the bug of ptrace attach and signal handling concurrency

[PATCH] signal/ptrace: Fix the bug of ptrace attach and signal handling concurrency

[Shuzhi Zu]

When ptrace attach and signal handling are concurrent, it will cause the
state of all threads of the traced process to become stopped and trace
pid is 0.

The thread dequeues the signal from the queue and another thread traces
the thread while the signal has not been handled or when non-real-time
signals less than SIGSTOP and a SIGSTOP signal are in the signal pending
queue at the same time, these two cases will cause all threads of the
process to be stopped. Therefore, when ptrace_attach modifying the task
ptrace value of the thread, it is necessary to lock. Except for synchronous
signals, SIGSTOP signal should be handled before non-real-time signals.

Example: Thread A is the traced thread. Thread B is the tracer.
1.Thread A dequeues the SIGCHLD signal, before it is handled, thread B
executes ptrace attach A, and A's ptrace is assigned PT_PTRACED. Then the
SIGCHLD signal processing will go to the ptrace_signal function。Thread A
becomes traced. Thread B continues to execute ptrace_detach after the
execution of waitpid returns. A thread ptrace value is set to 0 and
it is woken up. Thread A continues to process the SIGSTOP signal, causing
all threads of the process to be stopped.
2. Thread A ptrace is assigned PT_PTRACED by ptrace attach, and both
SIGPIPE and SIGSTOP signals exist in the pending queue of thread A.
Because the value of SIGPIPE is less than SIGSTOP, SIGSPIPE is handled
first, which will result in a timing similar to Example 1。

Example 1：
    A                             B
    get_signal
    dequeue_signal (SIGCHLD）
                                  ptrace_attach ( A->ptrace |= PT_PTRACED)

    ptrace_signal
    ->ptrace_stop(TASK_TRACED)
                                  ptrace_attach ( Send SIGSTOP to A)
                                  ptrace_waitpid( return 0)
                                  ptrace_detach (A->ptrace=0, wakeup A)
    dequeue_signal(SIGSTOP)
    sig_kernel_stop(SIGSTOP)
    do_signal_stop (TASK_STOPPED)

then：
A (other threads of the process received signal)
get_signal-> do_signal_stop(0))->TASK_STOPPED

Example 2：
    A                             B
    send_sig(SIGPIPE, current, 0)
                                  ptrace_attach ( A->ptrace |= PT_PTRACED)
                                  ptrace_attach ( Send SIGSTOP to A)
    get_signal
    dequeue_signal (SIGPIPE)
    ptrace_signal
    ->ptrace_stop(TASK_TRACED)

                                  ptrace_waitpid( return 0)
                                  ptrace_detach (A->ptrace=0, wakeup A)
    dequeue_signal(SIGSTOP)
    sig_kernel_stop(SIGSTOP)
    do_signal_stop (TASK_STOPPED)

then：
A (other threads of the process received signal)
get_signal-> do_signal_stop(0))->TASK_STOPPED

Signed-off-by: Shuzhi Zu <zushuzhi@h3c.com>
---
 kernel/ptrace.c | 2 ++
 kernel/signal.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index ccc4b46..ab1dc8f 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -447,7 +447,9 @@ static int ptrace_attach(struct task_struct *task, long request,
 	if (task->ptrace)
 		goto unlock_tasklist;
 
+	spin_lock(&task->sighand->siglock);
 	task->ptrace = flags;
+	spin_unlock(&task->sighand->siglock);
 
 	ptrace_link(task, current);
 
diff --git a/kernel/signal.c b/kernel/signal.c
index 30cd1ca..522bc6e 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -220,6 +220,8 @@ int next_signal(struct sigpending *pending, sigset_t *mask)
 	if (x) {
 		if (x & SYNCHRONOUS_MASK)
 			x &= SYNCHRONOUS_MASK;
+		else if (x & sigmask(SIGSTOP))
+			x &= sigmask(SIGSTOP);
 		sig = ffz(~x) + 1;
 		return sig;
 	}
-- 
1.8.3.1

Re: [PATCH] signal/ptrace: Fix the bug of ptrace attach and signal handling concurrency

[Oleg Nesterov]

Hi Shuzhi,

On 05/01, Shuzhi Zu wrote:
>
> Example 1：
>     A                             B
>     get_signal
>     dequeue_signal (SIGCHLD）
>                                   ptrace_attach ( A->ptrace |= PT_PTRACED)
>
>     ptrace_signal
>     ->ptrace_stop(TASK_TRACED)
>                                   ptrace_attach ( Send SIGSTOP to A)
>                                   ptrace_waitpid( return 0)
>                                   ptrace_detach (A->ptrace=0, wakeup A)
>     dequeue_signal(SIGSTOP)
>     sig_kernel_stop(SIGSTOP)
>     do_signal_stop (TASK_STOPPED)
>
> then：
> A (other threads of the process received signal)
> get_signal-> do_signal_stop(0))->TASK_STOPPED

Yes, there are a lot of known problems with send_sig_info(SIGSTOP) in
ptrace_attach(). This one of the reasons for PTRACE_SEIZE which doesn't
abuse SIGSTOP. Please use it instead of PTRACE_ATTACH, PTRACE_SEIZE has
more features.


As for your particular example, this is an application bug. Debugger (if
it uses PTRACE_ATTACH) should not detach until the tracee reports SIGSTOP
sent by PTRACE_ATTACH.

This can lead to other problems, say we can miss a "real" SIGSTOP from
another application, but again PTRACE_ATTACH is hopeless wrt SIGSTOP,
please consider PTRACE_SEIZE.

Oleg.