linux-kernel - [PATCH v3] locking/mutex: Set and clear owner using WRITE

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

Message-ID: <1464112677.2519.8.camel@j-VirtualBox>
Date:	Tue, 24 May 2016 10:57:57 -0700
From:	Jason Low <jason.low2@....com>
To:	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Waiman Long <waiman.long@....com>,
	Davidlohr Bueso <dave@...olabs.net>,
	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Terry Rudd <terry.rudd@....com>,
	Scott J Norton <scott.norton@....com>,
	Jason Low <jason.low2@...com>
Subject: [PATCH v3] locking/mutex: Set and clear owner using WRITE_ONCE()

The mutex owner can get read and written to locklessly.
Use WRITE_ONCE when setting and clearing the owner field
in order to avoid optimizations such as store tearing. This
avoids situations where the owner field gets written to with
multiple stores and another thread could concurrently read
and use a partially written owner value.

This is not necessary in the debug case where the owner gets
modified with the wait_lock held.

Signed-off-by: Jason Low <jason.low2@....com>
Acked-by: Davidlohr Bueso <dave@...olabs.net>
Acked-by: Waiman Long <Waiman.Long@....com>
---
 kernel/locking/mutex-debug.h |  5 +++++
 kernel/locking/mutex.h       | 10 ++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/mutex-debug.h b/kernel/locking/mutex-debug.h
index 0799fd3..abda3af 100644
--- a/kernel/locking/mutex-debug.h
+++ b/kernel/locking/mutex-debug.h
@@ -27,6 +27,11 @@ extern void debug_mutex_unlock(struct mutex *lock);
 extern void debug_mutex_init(struct mutex *lock, const char *name,
 			     struct lock_class_key *key);
 
+/*
+ * In the debug case, we don't need to use WRITE_ONCE
+ * to set and clear the owner, because the wait_lock
+ * is taken when reading and writing to the owner field.
+ */
 static inline void mutex_set_owner(struct mutex *lock)
 {
 	lock->owner = current;
diff --git a/kernel/locking/mutex.h b/kernel/locking/mutex.h
index 5cda397..12f9619 100644
--- a/kernel/locking/mutex.h
+++ b/kernel/locking/mutex.h
@@ -17,14 +17,20 @@
 		__list_del((waiter)->list.prev, (waiter)->list.next)
 
 #ifdef CONFIG_MUTEX_SPIN_ON_OWNER
+/*
+ * The mutex owner can get read and written to locklessly.
+ * We should use WRITE_ONCE when writing the owner value to
+ * avoid store tearing, otherwise, a thread could potentially
+ * read a partially written and incomplete owner value.
+ */
 static inline void mutex_set_owner(struct mutex *lock)
 {
-	lock->owner = current;
+	WRITE_ONCE(lock->owner, current);
 }
 
 static inline void mutex_clear_owner(struct mutex *lock)
 {
-	lock->owner = NULL;
+	WRITE_ONCE(lock->owner, NULL);
 }
 #else
 static inline void mutex_set_owner(struct mutex *lock)
-- 
2.1.4