linux-kernel - [PATCH 3.16 14/99] panic: avoid deadlocks in re-entrant console drivers

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]

Date:   Tue, 02 Apr 2019 14:38:27 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Sergey Senozhatsky" <sergey.senozhatsky@...il.com>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        "Daniel Wang" <wonderfly@...gle.com>,
        "Jiri Slaby" <jslaby@...e.com>, "Petr Mladek" <pmladek@...e.com>,
        "Sergey Senozhatsky" <sergey.senozhatsky.work@...il.com>,
        "Peter Feiner" <pfeiner@...gle.com>,
        "Alan Cox" <gnomes@...rguk.ukuu.org.uk>,
        linux-serial@...r.kernel.org,
        "Linus Torvalds" <torvalds@...ux-foundation.org>,
        "Peter Zijlstra" <peterz@...radead.org>,
        "Steven Rostedt" <rostedt@...dmis.org>
Subject: [PATCH 3.16 14/99] panic: avoid deadlocks in re-entrant console
 drivers

3.16.65-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>

commit c7c3f05e341a9a2bd1a92993d4f996cfd6e7348e upstream.

=46romprintk()/serial console point of view panic() is special, because
it may force CPU to re-enter printk() or/and serial console driver.
Therefore, some of serial consoles drivers are re-entrant. E.g. 8250:

serial8250_console_write()
{
	if (port->sysrq)
		locked = 0;
	else if (oops_in_progress)
		locked = spin_trylock_irqsave(&port->lock, flags);
	else
		spin_lock_irqsave(&port->lock, flags);
	...
}

panic() does set oops_in_progress via bust_spinlocks(1), so in theory
we should be able to re-enter serial console driver from panic():

	CPU0
	<NMI>
	uart_console_write()
	serial8250_console_write()		// if (oops_in_progress)
						//    spin_trylock_irqsave()
	call_console_drivers()
	console_unlock()
	console_flush_on_panic()
	bust_spinlocks(1)			// oops_in_progress++
	panic()
	<NMI/>
	spin_lock_irqsave(&port->lock, flags)   // spin_lock_irqsave()
	serial8250_console_write()
	call_console_drivers()
	console_unlock()
	printk()
	...

However, this does not happen and we deadlock in serial console on
port->lock spinlock. And the problem is that console_flush_on_panic()
called after bust_spinlocks(0):

void panic(const char *fmt, ...)
{
	bust_spinlocks(1);
	...
	bust_spinlocks(0);
	console_flush_on_panic();
	...
}

bust_spinlocks(0) decrements oops_in_progress, so oops_in_progress
can go back to zero. Thus even re-entrant console drivers will simply
spin on port->lock spinlock. Given that port->lock may already be
locked either by a stopped CPU, or by the very same CPU we execute
panic() on (for instance, NMI panic() on printing CPU) the system
deadlocks and does not reboot.

Fix this by removing bust_spinlocks(0), so oops_in_progress is always
set in panic() now and, thus, re-entrant console drivers will trylock
the port->lock instead of spinning on it forever, when we call them
from console_flush_on_panic().

Link: http://lkml.kernel.org/r/20181025101036.6823-1-sergey.senozhatsky@gmail.com
Cc: Steven Rostedt <rostedt@...dmis.org>
Cc: Daniel Wang <wonderfly@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Alan Cox <gnomes@...rguk.ukuu.org.uk>
Cc: Jiri Slaby <jslaby@...e.com>
Cc: Peter Feiner <pfeiner@...gle.com>
Cc: linux-serial@...r.kernel.org
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Signed-off-by: Petr Mladek <pmladek@...e.com>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 kernel/panic.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/kernel/panic.c
+++ b/kernel/panic.c
@@ -13,6 +13,7 @@
 #include <linux/kmsg_dump.h>
 #include <linux/kallsyms.h>
 #include <linux/notifier.h>
+#include <linux/vt_kern.h>
 #include <linux/module.h>
 #include <linux/random.h>
 #include <linux/ftrace.h>
@@ -178,7 +179,10 @@ void panic(const char *fmt, ...)
 	 */
 	crash_kexec(NULL);
 
-	bust_spinlocks(0);
+#ifdef CONFIG_VT
+	unblank_screen();
+#endif
+	console_unblank();
 
 	/*
 	 * We may have ended up stopping the CPU holding the lock (in