linux-kernel - [PATCH v6 2/2] printk: Make printing of spin

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: <1457692627-14076-3-git-send-email-byungchul.park@lge.com>
Date:	Fri, 11 Mar 2016 19:37:07 +0900
From:	Byungchul Park <byungchul.park@....com>
To:	akpm@...ux-foundation.org, mingo@...nel.org
Cc:	linux-kernel@...r.kernel.org, akinobu.mita@...il.com, jack@...e.cz,
	sergey.senozhatsky.work@...il.com, peter@...leysoftware.com,
	tglx@...utronix.de, peterz@...radead.org, rostedt@...dmis.org
Subject: [PATCH v6 2/2] printk: Make printing of spin_dump() deferred to avoid a deadlock

In the case that console_sem.lock has already been held, printk() must
not be performed in normal way, otherwise a deadlock can happen because
it tries to acquire the console_sem.lock again, e.i. deadlock. This
patch defers the actual printing of printk(), using irq_work_queue().

The scenario the bad thing can happen is,

printk
  console_trylock
  console_unlock
    up_console_sem
      up
        raw_spin_lock_irqsave(&sem->lock, flags)
        __up
          wake_up_process
            try_to_wake_up
              raw_spin_lock_irqsave(&p->pi_lock)
                __spin_lock_debug
                  spin_dump
                    printk
                      console_trylock
                        raw_spin_lock_irqsave(&sem->lock, flags)
                        >>> DEADLOCK <<<

Signed-off-by: Byungchul Park <byungchul.park@....com>
---
 kernel/locking/spinlock_debug.c | 33 ++++++++++++++++++++++++++++++++-
 kernel/printk/printk.c          |  8 ++++++++
 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/spinlock_debug.c b/kernel/locking/spinlock_debug.c
index 0374a59..fd24588 100644
--- a/kernel/locking/spinlock_debug.c
+++ b/kernel/locking/spinlock_debug.c
@@ -49,7 +49,7 @@ void __rwlock_init(rwlock_t *lock, const char *name,
 
 EXPORT_SYMBOL(__rwlock_init);
 
-static void spin_dump(raw_spinlock_t *lock, const char *msg)
+static void __spin_dump(raw_spinlock_t *lock, const char *msg)
 {
 	struct task_struct *owner = NULL;
 
@@ -67,6 +67,37 @@ static void spin_dump(raw_spinlock_t *lock, const char *msg)
 	dump_stack();
 }
 
+extern int console_sem_spin_is_held(void);
+
+static void __spin_dump_deferred(raw_spinlock_t *lock, const char *msg)
+{
+	printk_func_t s;
+
+	s = this_cpu_read(printk_func);
+	this_cpu_write(printk_func, vprintk_deferred);
+
+	/*
+	 * To change printk_func, it must be in preempt disabled and irq
+	 * disabled. WARN_ON() should be called after the change because
+	 * the default printk_func which may be called from WARN_ON()
+	 * is prohibited in this context.
+	 */
+	WARN_ON(!preempt_count() || !irqs_disabled());
+	__spin_dump(lock, msg);
+
+	this_cpu_write(printk_func, s);
+
+	printk_pending_output();
+}
+
+static void spin_dump(raw_spinlock_t *lock, const char *msg)
+{
+	if (unlikely(console_sem_spin_is_held()))
+		__spin_dump_deferred(lock, msg);
+	else
+		__spin_dump(lock, msg);
+}
+
 static void spin_bug(raw_spinlock_t *lock, const char *msg)
 {
 	if (!debug_locks_off())
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index e82fae4..2e6b008 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2188,6 +2188,14 @@ int is_console_locked(void)
 	return console_locked;
 }
 
+#ifdef CONFIG_DEBUG_SPINLOCK
+int console_sem_spin_is_held(void)
+{
+	return raw_spin_is_locked(&console_sem.lock) &&
+			(console_sem.lock.owner == current);
+}
+#endif
+
 static void console_cont_flush(char *text, size_t size)
 {
 	unsigned long flags;
-- 
1.9.1