linux-kernel - [PATCH] posix-timers: Fix a race with __exit

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>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241202155547.8214-1-iii@linux.ibm.com>
Date: Mon,  2 Dec 2024 16:54:32 +0100
From: Ilya Leoshkevich <iii@...ux.ibm.com>
To: Thomas Gleixner <tglx@...utronix.de>
Cc: LKML <linux-kernel@...r.kernel.org>,
        Frederic Weisbecker <frederic@...nel.org>,
        Oleg Nesterov <oleg@...hat.com>, Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Richard Henderson <richard.henderson@...aro.org>,
        Ilya Leoshkevich <iii@...ux.ibm.com>
Subject: [PATCH] posix-timers: Fix a race with __exit_signal()

If a thread exit happens simultaneously with a POSIX timer signal, this
POSIX timer may end up being erroneously stopped. This problem may be
reproduced with a small C program that creates a POSIX timer and
constantly respawns threads, e.g. [1].

When posixtimer_send_sigqueue() races against __exit_signal(), the
latter may clear the selected task's sighand, causing
lock_task_sighand() to fail. This is possible because __exit_signal()
clears the task's sighand under the sighand lock, but
lock_task_sighand() needs to first load it without taking this lock.

The it_sigqueue_seq update needs to happen under the sighand lock;
failure to take this lock means that it is not possible to make that
update. And if it_sigqueue_seq is not updated, then
__posixtimer_deliver_signal() does not rearm the timer, effectively
stopping it.

Fix by choosing another thread if the one chosen by
posixtimer_get_target() is exiting. This requires taking tasklist_lock,
which is ordered before the sighand lock, as can be seen in, e.g.,
release_task(). tasklist_lock may be released immediately after the
sighand lock is successfully taken, which will look nicer, but will
create uncertainty w.r.t. restoring the irq flags, so release it
at the end of posixtimer_send_sigqueue().

There is another user of posixtimer_get_target(), which may potentially
be affected as well: posixtimer_sig_unignore(). But it is called with
the sighand lock taken, so the race with __exit_signal() is fortunately
not possible there.

[1] https://gitlab.com/qemu-project/qemu/-/blob/v9.1.1/tests/tcg/multiarch/signals.c?ref_type=tags

Signed-off-by: Ilya Leoshkevich <iii@...ux.ibm.com>
---
 kernel/signal.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 989b1cc9116a..ff1608997301 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1974,10 +1974,11 @@ static inline struct task_struct *posixtimer_get_target(struct k_itimer *tmr)

 void posixtimer_send_sigqueue(struct k_itimer *tmr)
 {
+	unsigned long flags, tasklist_flags;
 	struct sigqueue *q = &tmr->sigq;
+	bool tasklist_locked = false;
 	int sig = q->info.si_signo;
 	struct task_struct *t;
-	unsigned long flags;
 	int result;

 	guard(rcu)();
@@ -1986,8 +1987,25 @@ void posixtimer_send_sigqueue(struct k_itimer *tmr)
 	if (!t)
 		return;

-	if (!likely(lock_task_sighand(t, &flags)))
-		return;
+	if (!likely(lock_task_sighand(t, &flags))) {
+		/*
+		 * The target is exiting, pick another one. This ensures that
+		 * it_sigqueue_seq is updated, otherwise
+		 * posixtimer_deliver_signal() will not rearm the timer.
+		 */
+		bool found = false;
+
+		read_lock_irqsave(&tasklist_lock, tasklist_flags);
+		tasklist_locked = true;
+		do_each_pid_task(tmr->it_pid, tmr->it_pid_type, t) {
+			if (likely(lock_task_sighand(t, &flags))) {
+				found = true;
+				break;
+			}
+		} while_each_pid_task(tmr->it_pid, tmr->it_pid_type, t);
+		if (!likely(found))
+			goto out_tasklist;
+	}

 	/*
 	 * Update @tmr::sigqueue_seq for posix timer signals with sighand
@@ -2062,6 +2080,9 @@ void posixtimer_send_sigqueue(struct k_itimer *tmr)
 out:
 	trace_signal_generate(sig, &q->info, t, tmr->it_pid_type != PIDTYPE_PID, result);
 	unlock_task_sighand(t, &flags);
+out_tasklist:
+	if (tasklist_locked)
+		read_unlock_irqrestore(&tasklist_lock, tasklist_flags);
 }

 static inline void posixtimer_sig_ignore(struct task_struct *tsk, struct sigqueue *q)
-- 
2.47.0