linux-kernel - [PATCH v2 3/3] tty: Use RCU read lock to iterate tasks and threads in __do

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

Message-ID: <151619279470.5683.15554197416743431023.stgit@localhost.localdomain>
Date:   Wed, 17 Jan 2018 15:39:54 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     gregkh@...uxfoundation.org, jslaby@...e.com, oleg@...hat.com,
        ebiederm@...ssion.com, ktkhai@...tuozzo.com
Cc:     linux-kernel@...r.kernel.org
Subject: [PATCH v2 3/3] tty: Use RCU read lock to iterate tasks and threads
 in __do_SAK()

There were made several efforts to make __do_SAK()
working in process context long ago, but it does
not solves the problem completely. Since __do_SAK()
may take tasklist_lock for a long time, the concurent
processes, waiting for write lock with interrupts
disabled (e.g., forking), get into the same situation
like __do_SAK() would have been executed in interrupt
context. I've observed several hard lockups on 3.10
kernel running 200 containers, caused by long duration
of copy_process()->write_lock_irq() after SAK was sent
to a tty. Current mainline kernel has the same problem.

The solution is to use RCU to iterate processes and threads.
Task list integrity is the only reason we taken tasklist_lock
before, as tty subsys primitives mostly take it for reading
also (e.g., __proc_set_tty). RCU read lock is enough for that.
This patch solves the problem and makes __do_SAK() to be
not greedy of tasklist_lock. That should prevent hard lockups
I've pointed above.

Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>
---
 drivers/tty/tty_io.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 89326cee2403..55115e65668d 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2724,7 +2724,9 @@ void __do_SAK(struct tty_struct *tty)
 			   task_pid_nr(p), p->comm);
 		send_sig(SIGKILL, p, 1);
 	} while_each_pid_task(session, PIDTYPE_SID, p);
+	read_unlock(&tasklist_lock);

+	rcu_read_lock();
 	/* Now kill any processes that happen to have the tty open */
 	for_each_process(p) {
 		if (p->signal->tty == tty) {
@@ -2754,7 +2756,7 @@ void __do_SAK(struct tty_struct *tty)
 kill:
 		send_sig(SIGKILL, p, 1);
 	}
-	read_unlock(&tasklist_lock);
+	rcu_read_unlock();
 #endif
 }