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:	Fri, 28 Jun 2013 22:30:36 +0000 (GMT)
From:	jbaron@...mai.com
To:	rostedt@...dmis.org, andi@...stfloor.org
Cc:	linux-kernel@...r.kernel.org, mingo@...nel.org,
	peterz@...radead.org
Subject: [PATCH 1/3] static_keys: Add a static_key_slow_set_true()/false() interface

As pointed out by Andi Kleen, some static key users can be racy because they
check the value of the key->enabled, and then subsequently update the branch
direction. A number of call sites have 'higher' level locking that avoids this
race, but the usage in the scheduler features does not. See:
http://lkml.indiana.edu/hypermail/linux/kernel/1304.2/01655.html

Thus, introduce a new API that does the check and set under the
'jump_label_mutex'. This should also allow to simplify call sites a bit.

Users of static keys should use either the inc/dec or the set_true/set_false
API.

Signed-off-by: Jason Baron <jbaron@...mai.com>
---
 Documentation/static-keys.txt |    8 ++++++++
 include/linux/jump_label.h    |   30 ++++++++++++++++++++++++++++++
 kernel/jump_label.c           |   40 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)

diff --git a/Documentation/static-keys.txt b/Documentation/static-keys.txt
index 9f5263d..4cca941 100644
--- a/Documentation/static-keys.txt
+++ b/Documentation/static-keys.txt
@@ -134,6 +134,14 @@ An example usage in the kernel is the implementation of tracepoints:
                                 TP_CONDITION(cond));                    \
         }
 
+Keys can also be updated with the following calls:
+
+	static_key_slow_set_true(struct static_key *key);
+	static_key_slow_set_false(struct static_key *key);
+
+Users should of the API should not mix the inc/dec with usages
+of set_true/set_false. That is, users should choose one or the other.
+
 Tracepoints are disabled by default, and can be placed in performance critical
 pieces of the kernel. Thus, by using a static key, the tracepoints can have
 absolutely minimal impact when not in use.
diff --git a/include/linux/jump_label.h b/include/linux/jump_label.h
index 0976fc4..787ab73 100644
--- a/include/linux/jump_label.h
+++ b/include/linux/jump_label.h
@@ -67,6 +67,11 @@ struct static_key_deferred {
 	struct delayed_work work;
 };
 
+/* for use with set_true/set_false API */
+struct static_key_boolean {
+	struct static_key key;
+};
+
 # include <asm/jump_label.h>
 # define HAVE_JUMP_LABEL
 #endif	/* CC_HAVE_ASM_GOTO && CONFIG_JUMP_LABEL */
@@ -120,6 +125,8 @@ extern int jump_label_text_reserved(void *start, void *end);
 extern void static_key_slow_inc(struct static_key *key);
 extern void static_key_slow_dec(struct static_key *key);
 extern void static_key_slow_dec_deferred(struct static_key_deferred *key);
+extern void static_key_slow_set_true(struct static_key_boolean *key);
+extern void static_key_slow_set_false(struct static_key_boolean *key);
 extern void jump_label_apply_nops(struct module *mod);
 extern void
 jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
@@ -128,6 +135,10 @@ jump_label_rate_limit(struct static_key_deferred *key, unsigned long rl);
 	{ .enabled = ATOMIC_INIT(1), .entries = (void *)1 })
 #define STATIC_KEY_INIT_FALSE ((struct static_key) \
 	{ .enabled = ATOMIC_INIT(0), .entries = (void *)0 })
+#define STATIC_KEY_BOOLEAN_INIT_TRUE ((struct static_key_boolean) \
+	{ .key.enabled = ATOMIC_INIT(1), .key.entries = (void *)1 })
+#define STATIC_KEY_BOOLEAN_INIT_FALSE ((struct static_key_boolean) \
+	{ .key.enabled = ATOMIC_INIT(0), .key.entries = (void *)0 })
 
 #else  /* !HAVE_JUMP_LABEL */
 
@@ -137,6 +148,11 @@ struct static_key {
 	atomic_t enabled;
 };
 
+/* for use with set_true/set_false API */
+struct static_key_boolean {
+	struct static_key key;
+};
+
 static __always_inline void jump_label_init(void)
 {
 }
@@ -174,6 +190,16 @@ static inline void static_key_slow_dec_deferred(struct static_key_deferred *key)
 	static_key_slow_dec(&key->key);
 }
 
+static inline void static_key_slow_set_true(struct static_key_boolean *key)
+{
+	atomic_set(&key->key.enabled, 1);
+}
+
+static inline void static_key_slow_set_false(struct static_key_boolean *key)
+{
+	atomic_set(&key->key.enabled, 0);
+}
+
 static inline int jump_label_text_reserved(void *start, void *end)
 {
 	return 0;
@@ -197,6 +223,10 @@ jump_label_rate_limit(struct static_key_deferred *key,
 		{ .enabled = ATOMIC_INIT(1) })
 #define STATIC_KEY_INIT_FALSE ((struct static_key) \
 		{ .enabled = ATOMIC_INIT(0) })
+#define STATIC_KEY_BOOLEAN_INIT_TRUE ((struct static_key_boolean) \
+		{ .key.enabled = ATOMIC_INIT(1) })
+#define STATIC_KEY_BOOLEAN_INIT_FALSE ((struct static_key_boolean) \
+		{ .key.enabled = ATOMIC_INIT(0) })
 
 #endif	/* HAVE_JUMP_LABEL */
 
diff --git a/kernel/jump_label.c b/kernel/jump_label.c
index 60f48fa..2234a4c 100644
--- a/kernel/jump_label.c
+++ b/kernel/jump_label.c
@@ -120,6 +120,46 @@ void jump_label_rate_limit(struct static_key_deferred *key,
 }
 EXPORT_SYMBOL_GPL(jump_label_rate_limit);
 
+void static_key_slow_set_true(struct static_key_boolean *key_boolean)
+{
+	struct static_key *key = (struct static_key *)key_boolean;
+	int enabled;
+
+	jump_label_lock();
+	enabled = atomic_read(&key->enabled);
+	if (enabled == 1)
+		goto out;
+	WARN(enabled != 0, "%s: key->enabled: %d\n", __func__, enabled);
+	if (!jump_label_get_branch_default(key))
+		jump_label_update(key, JUMP_LABEL_ENABLE);
+	else
+		jump_label_update(key, JUMP_LABEL_DISABLE);
+	atomic_set(&key->enabled, 1);
+out:
+	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_slow_set_true);
+
+void static_key_slow_set_false(struct static_key_boolean *key_boolean)
+{
+	struct static_key *key = (struct static_key *)key_boolean;
+	int enabled;
+
+	jump_label_lock();
+	enabled = atomic_read(&key->enabled);
+	if (enabled == 0)
+		goto out;
+	WARN(enabled != 1, "%s: key->enabled: %d\n", __func__, enabled);
+	if (!jump_label_get_branch_default(key))
+		jump_label_update(key, JUMP_LABEL_DISABLE);
+	else
+		jump_label_update(key, JUMP_LABEL_ENABLE);
+	atomic_set(&key->enabled, 0);
+out:
+	jump_label_unlock();
+}
+EXPORT_SYMBOL_GPL(static_key_slow_set_false);
+
 static int addr_conflict(struct jump_entry *entry, void *start, void *end)
 {
 	if (entry->code <= (unsigned long)end &&
-- 
1.7.9.5

--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ