linux-kernel - [PATCH] signal/ptrace: Fix the problem of ptrace attach and signal handling oncurrency

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Sat, 30 Apr 2022 23:43:57 +0800
From:   Shuzhi Zu <zushuzhi@....com>
To:     <brauner@...nel.org>
CC:     <oleg@...hat.com>, <linux-kernel@...r.kernel.org>,
        Shuzhi Zu <zushuzhi@....com>
Subject: [PATCH] signal/ptrace: Fix the problem of ptrace attach and signal handling oncurrency

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@....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