linux-kernel - [PATCH] printk: Fix spinlock deadlock in printk reenty

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: <1480490119-63559-1-git-send-email-linyongting@huawei.com>
Date:   Wed, 30 Nov 2016 15:15:19 +0800
From:   <linyongting@...wei.com>
To:     <kejinling@...wei.com>, <akpm@...ux-foundation.org>,
        <pmladek@...e.com>, <sergey.senozhatsky@...il.com>, <bp@...e.de>,
        <tj@...nel.org>, <treding@...dia.com>,
        <linux-kernel@...r.kernel.org>, <linyongting@...wei.com>
CC:     <leisure.wang@...wei.com>
Subject: [PATCH] printk: Fix spinlock deadlock in printk reenty

From: Jinling Ke <kejinling@...wei.com>

when Oops in printk, printk will call zap_locks() to reinitialize
spinlock to prevent deadlock. In arm, arm64, x86 or other
architecture smp cpu, race condition will occur in printk spinlock
logbuf_lock and then it will result other cpu that is waiting printk
spinlock in deadlock(in function raw_spin_lock). Because the cpus
deadlock, you can see the error  printk log:

"SMP: failed to stop secondary CPUs"

In arm, arm64, x86 or other architecture, spinlock variable
is divided into 2 parts, for example they are 'owner' and 'next' in arm.
When get a spinlock, the 'next' part will add 1 and wait 'next' being
equal to 'owner'. However, at this moment, the 'next' part is local
variable, but 'owner' part value is get from global variable logbuf_lock.
However,raw_spin_lock_init(&logbuf_lock) will set 'owner' part and
'next' part to zero, the result is that cpu deadlock in function
raw_spin_lock( while loop in function arch_spin_lock ).

	struct of arm spinlock
	 	union {
			u32 slock;
			struct __raw_tickets {
				u16 owner;
				u16 next;
			} tickets;
		};
	} arch_spinlock_t;
	static inline void arch_spin_lock(arch_spinlock_t *lock)
	{...
		<--- At the moment, other cpu call zap_locks()->spin_lock_init(),
		<--- set the 'owner' part to zero, but lockval.tickets.next is a
	        <--- local variable
		while (lockval.tickets.next != lockval.tickets.owner) {
			lockval.tickets.owner = ACCESS_ONCE(lock->tickets.owner);
		}
	...
	}

The solution is that In function zap_locks(), replace
raw_spin_lock_init(&logbuf_lock) with raw_spin_unlock(&logbuf_lock),
to let spin_lock stay in unlocked.

Signed-off-by: Jinling Ke <kejinling@...wei.com>
---
 kernel/printk/printk.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index f7a55e9..05b1886 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1603,7 +1603,7 @@ static void zap_locks(void)

 	debug_locks_off();
 	/* If a crash is occurring, make sure we can't deadlock */
-	raw_spin_lock_init(&logbuf_lock);
+	raw_spin_unlock(&logbuf_lock);
 	/* And make sure that we print immediately */
 	sema_init(&console_sem, 1);
 }
-- 
1.7.9.5