linux-kernel - Re: [PATCH v4 16/18] static

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: <20200506211547.2vnuhkfzhgbi27dm@treble>
Date:   Wed, 6 May 2020 16:15:47 -0500
From:   Josh Poimboeuf <jpoimboe@...hat.com>
To:     Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, rostedt@...dmis.org,
        mhiramat@...nel.org, bristot@...hat.com, jbaron@...mai.com,
        torvalds@...ux-foundation.org, tglx@...utronix.de,
        mingo@...nel.org, namit@...are.com, hpa@...or.com, luto@...nel.org,
        ard.biesheuvel@...aro.org, pbonzini@...hat.com,
        mathieu.desnoyers@...icios.com
Subject: Re: [PATCH v4 16/18] static_call: Allow early init

On Fri, May 01, 2020 at 10:29:05PM +0200, Peter Zijlstra wrote:
> In order to use static_call() to wire up x86_pmu, we need to
> initialize earlier; copy some of the tricks from jump_label to enable
> this.
> 
> Primarily we overload key->next to store a sites pointer when there
> are no modules, this avoids having to use kmalloc() to initialize the
> sites and allows us to run much earlier.
> 
> (arguably, this is much much earlier than needed for perf, but it
> might allow other uses.)
> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
> ---
>  arch/x86/kernel/setup.c       |    2 +
>  arch/x86/kernel/static_call.c |    8 +++++-
>  include/linux/static_call.h   |   15 ++++++++++--
>  kernel/static_call.c          |   52 +++++++++++++++++++++++++++++++++++++++---
>  4 files changed, 71 insertions(+), 6 deletions(-)

This doesn't work when the key is defined in a module.  In
__static_call_update(), first.site_mod->mod is NULL, but
static_call_key_sites() points to the module's call sites.

This seems to fix it (sorry, also has the 'next' -> 'mods' rename).
The actual fix is in static_call_key_sites() and static_call_key_mods().

diff --git a/include/linux/static_call.h b/include/linux/static_call.h
index e112553cfce6..ee55075707f1 100644
--- a/include/linux/static_call.h
+++ b/include/linux/static_call.h
@@ -110,9 +110,10 @@ struct static_call_mod {
 struct static_call_key {
 	void *func;
 	union {
-		/* bit0 => 0 - next, 1 - sites */
+		/* bit 0: 0 = mods; 1 = sites */
 		unsigned long type;
-		struct static_call_mod *next;
+
+		struct static_call_mod *mods;
 		struct static_call_site *sites;
 	};
 };
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 783953db28c4..d78e50c5b8a3 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -94,23 +94,23 @@ static inline void static_call_sort_entries(struct static_call_site *start,
 	     static_call_site_cmp, static_call_site_swap);
 }
 
-static inline bool static_call_key_has_next(struct static_call_key *key)
+static inline bool static_call_key_has_mods(struct static_call_key *key)
 {
 	return !(key->type & 1);
 }
 
-static inline struct static_call_mod *static_call_key_next(struct static_call_key *key)
+static inline struct static_call_mod *static_call_key_mods(struct static_call_key *key)
 {
-	if (static_call_key_has_next(key))
-		return key->next->next;
+	if (!static_call_key_has_mods(key))
+		return NULL;
 
-	return NULL;
+	return key->mods;
 }
 
 static inline struct static_call_site *static_call_key_sites(struct static_call_key *key)
 {
-	if (static_call_key_has_next(key))
-		return key->next->sites;
+	if (static_call_key_has_mods(key))
+		return NULL;
 
 	return (struct static_call_site *)(key->type & ~1);
 }
@@ -118,7 +118,12 @@ static inline struct static_call_site *static_call_key_sites(struct static_call_
 void __static_call_update(struct static_call_key *key, void *tramp, void *func)
 {
 	struct static_call_site *site, *stop;
-	struct static_call_mod *site_mod, first;
+	struct static_call_mod *site_mod;
+	struct static_call_mod first = {
+		.next = static_call_key_mods(key),
+		.mod = NULL,
+		.sites = static_call_key_sites(key),
+	};
 
 	cpus_read_lock();
 	static_call_lock();
@@ -137,17 +142,14 @@ void __static_call_update(struct static_call_key *key, void *tramp, void *func)
 	if (WARN_ON_ONCE(!static_call_initialized))
 		goto done;
 
-	first = (struct static_call_mod){
-		.next = static_call_key_next(key),
-		.mod = NULL,
-		.sites = static_call_key_sites(key),
-	};
-
 	for (site_mod = &first; site_mod; site_mod = site_mod->next) {
 		if (!site_mod->sites) {
 			/*
 			 * This can happen if the static call key is defined in
 			 * a module which doesn't use it.
+			 *
+			 * It also happens in the has_mods case, where the
+			 * 'first' entry has no sites associated with it.
 			 */
 			continue;
 		}
@@ -228,12 +230,12 @@ static int __static_call_init(struct module *mod,
 			if (!site_mod)
 				return -ENOMEM;
 
-			if (!static_call_key_has_next(key)) {
+			if (!static_call_key_has_mods(key)) {
 				site_mod->mod = NULL;
 				site_mod->next = NULL;
 				site_mod->sites = static_call_key_sites(key);
 
-				key->next = site_mod;
+				key->mods = site_mod;
 
 				site_mod = kzalloc(sizeof(*site_mod), GFP_KERNEL);
 				if (!site_mod)
@@ -242,8 +244,8 @@ static int __static_call_init(struct module *mod,
 
 			site_mod->mod = mod;
 			site_mod->sites = site;
-			site_mod->next = key->next;
-			key->next = site_mod;
+			site_mod->next = key->mods;
+			key->mods = site_mod;
 		}
 
 do_transform:
@@ -328,7 +330,7 @@ static void static_call_del_module(struct module *mod)
 
 		prev_key = key;
 
-		for (prev = &key->next, site_mod = key->next;
+		for (prev = &key->mods, site_mod = key->mods;
 		     site_mod && site_mod->mod != mod;
 		     prev = &site_mod->next, site_mod = site_mod->next)
 			;