linux-kernel - [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT

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:	Wed, 3 Apr 2013 19:41:22 +0530
From:	Vineet Gupta <Vineet.Gupta1@...opsys.com>
To:	<tglx@...utronix.de>
CC:	Vineet Gupta <Vineet.Gupta1@...opsys.com>,
	Christian Ruppert <christian.ruppert@...lis.com>,
	Pierrick Hascoet <pierrick.hascoet@...lis.com>,
	Robert Love <rml@...h9.net>,
	<kpreempt-tech@...ts.sourceforge.net>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Steven Rostedt <srostedt@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: [PATCH] [PATCH] Gaurantee spinlocks implicit barrier for !PREEMPT_COUNT

spinlocks built in a !PREEMPT_COUNT config don't have the compiler
barrier provided by preempt_* routines. This can break lot of code which
relies on barrier semantics.

This manifested as random crashes in timer code when stress testing
ARC Linux (3.9-rc3): !SMP && !PREEMPT_COUNT

Here's the exact sequence which caused this:
(0). tv1[x] <----> t1 <---> t2
(1). mod_timer(t1) interrupted after it calls timer_pending()
(2). mod_timer(t2) completes
(3). mod_timer(t1) resumes but messes up the list.
(4). __runt_timers( ) uses bogus timer_list entry / crashes in
     timer->function

when mod_timer() races against itself, the spinlock rightly serializes
the tv1[] timer link list, however timer_pending() called outside the
spinlock accesses timer's link list element, cached in a register.
With low register pressure (and a deep register file), there's nothing
forcing gcc to reload the element across the spinlock, causing a stale
value in register in case of race - ensuing a list corruption.

And the ARcompact disassembly which shows the culprit generated code:

mod_timer:
    push_s blink
    mov_s r13,r0	# timer, timer

..
    ###### timer_pending( )
    ld_s r3,[r13]       # <------ <variable>.entry.next LOADED
    brne r3, 0, @.L163

.L163:
..
    ###### spin_lock_irq( )
    lr  r5, [status32]  # flags
    bic r4, r5, 6       # temp, flags,
    and.f 0, r5, 6      # flags,
    flag.nz r4

    ###### detach_if_pending( ) begins

    tst_s r3,r3  <--------------
			# timer_pending( ) checks timer->entry.next
                        # r3 is NOT reloaded by gcc, using stale value
    beq.d @.L169
    mov.eq r0,0

    #####  detach_timer( ): __list_del( )

    ld r4,[r13,4]    	# <variable>.entry.prev, D.31439
    st r4,[r3,4]     	# <variable>.prev, D.31439

    st r3,[r4]       	# <variable>.next, D.30246

Signed-off-by: Vineet Gupta <vgupta@...opsys.com>
Reported-by: Christian Ruppert <christian.ruppert@...lis.com>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Christian Ruppert <christian.ruppert@...lis.com>
Cc: Pierrick Hascoet <pierrick.hascoet@...lis.com>
Cc: Robert Love <rml@...h9.net>
Cc: kpreempt-tech@...ts.sourceforge.net
Cc: Frederic Weisbecker <fweisbec@...il.com>
Cc: Steven Rostedt <srostedt@...hat.com>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Ingo Molnar <mingo@...nel.org>
Cc: linux-kernel@...r.kernel.org
---
 include/linux/preempt.h |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/linux/preempt.h b/include/linux/preempt.h
index 5a710b9..354d6e3 100644
--- a/include/linux/preempt.h
+++ b/include/linux/preempt.h
@@ -93,14 +93,19 @@ do { \
 
 #else /* !CONFIG_PREEMPT_COUNT */
 
-#define preempt_disable()		do { } while (0)
-#define sched_preempt_enable_no_resched()	do { } while (0)
-#define preempt_enable_no_resched()	do { } while (0)
-#define preempt_enable()		do { } while (0)
-
-#define preempt_disable_notrace()		do { } while (0)
-#define preempt_enable_no_resched_notrace()	do { } while (0)
-#define preempt_enable_notrace()		do { } while (0)
+/*
+ * compiler barrier needed to ensure that spinlocks provide the barrier
+ * semantics despite !CONFIG_PREEMPT_COUNT.
+ * See commit log for actual bug which forced this change
+ */
+#define preempt_disable()			do { barrier(); } while (0)
+#define sched_preempt_enable_no_resched()	do { barrier(); } while (0)
+#define preempt_enable_no_resched()		do { barrier(); } while (0)
+#define preempt_enable()			do { barrier(); } while (0)
+
+#define preempt_disable_notrace()		do { barrier(); } while (0)
+#define preempt_enable_no_resched_notrace()	do { barrier(); } while (0)
+#define preempt_enable_notrace()		do { barrier(); } while (0)
 
 #endif /* CONFIG_PREEMPT_COUNT */
 
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/