Re: [RFC PATCH 6/8] preempt/dynamic: Provide preempt_schedule[_notrace]() static calls

[Peter Zijlstra <peterz@infradead.org>]

On Wed, Jan 27, 2021 at 05:18:37PM -0600, Josh Poimboeuf wrote:

> +static struct static_call_tramp_key *tramp_key_lookup(unsigned long addr)
> +{
> +	struct static_call_tramp_key *start = __start_static_call_tramp_key;
> +	struct static_call_tramp_key *stop = __stop_static_call_tramp_key;
> +	struct static_call_tramp_key *tramp_key;
> +
> +	for (tramp_key = start; tramp_key != stop; tramp_key++) {
> +		unsigned long tramp;
> +
> +		tramp = (long)tramp_key->tramp + (long)&tramp_key->tramp;
> +		if (tramp == addr)
> +			return tramp_key;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int static_call_add_module(struct module *mod)
>  {
> -	return __static_call_init(mod, mod->static_call_sites,
> -				  mod->static_call_sites + mod->num_static_call_sites);
> +	struct static_call_site *start = mod->static_call_sites;
> +	struct static_call_site *stop = start + mod->num_static_call_sites;
> +	struct static_call_site *site;
> +
> +	for (site = start; site != stop; site++) {
> +		unsigned long addr = (unsigned long)static_call_key(site);
> +		struct static_call_tramp_key *tramp_key;
> +
> +		/*
> +		 * Is the key is exported, 'addr' points to the key, which
> +		 * means modules are allowed to call static_call_update() on
> +		 * it.
> +		 *
> +		 * Otherwise, the key isn't exported, and 'addr' points to the
> +		 * trampoline so we need to lookup the key.
> +		 *
> +		 * We go through this dance to prevent crazy modules from
> +		 * abusing sensitive static calls.
> +		 */
> +		if (!kernel_text_address(addr))
> +			continue;
> +
> +		tramp_key = tramp_key_lookup(addr);
> +		if (!tramp_key) {
> +			pr_warn("Failed to fixup __raw_static_call() usage at: %ps\n",
> +				static_call_addr(site));
> +			return -EINVAL;
> +		}
> +
> +		site->key = ((long)tramp_key->key - (long)&tramp_key->key) |
> +			    (site->key & STATIC_CALL_SITE_FLAGS);
> +	}
> +
> +	return __static_call_init(mod, start, stop);
>  }

I find it works better with this on..

---
diff --git a/kernel/static_call.c b/kernel/static_call.c
index 5e6f567976c1..6906c6ec4c97 100644
--- a/kernel/static_call.c
+++ b/kernel/static_call.c
@@ -325,7 +325,7 @@ static int __static_call_mod_text_reserved(void *start, void *end)
 	return ret;
 }
 
-static struct static_call_tramp_key *tramp_key_lookup(unsigned long addr)
+static unsigned long tramp_key_lookup(unsigned long addr)
 {
 	struct static_call_tramp_key *start = __start_static_call_tramp_key;
 	struct static_call_tramp_key *stop = __stop_static_call_tramp_key;
@@ -336,10 +336,10 @@ static struct static_call_tramp_key *tramp_key_lookup(unsigned long addr)
 
 		tramp = (long)tramp_key->tramp + (long)&tramp_key->tramp;
 		if (tramp == addr)
-			return tramp_key;
+			return (long)tramp_key->key + (long)&tramp_key->key;
 	}
 
-	return NULL;
+	return 0;
 }
 
 static int static_call_add_module(struct module *mod)
@@ -350,7 +350,7 @@ static int static_call_add_module(struct module *mod)
 
 	for (site = start; site != stop; site++) {
 		unsigned long addr = (unsigned long)static_call_key(site);
-		struct static_call_tramp_key *tramp_key;
+		unsigned long key;
 
 		/*
 		 * Is the key is exported, 'addr' points to the key, which
@@ -366,14 +366,14 @@ static int static_call_add_module(struct module *mod)
 		if (!kernel_text_address(addr))
 			continue;
 
-		tramp_key = tramp_key_lookup(addr);
-		if (!tramp_key) {
+		key = tramp_key_lookup(addr);
+		if (!key) {
 			pr_warn("Failed to fixup __raw_static_call() usage at: %ps\n",
 				static_call_addr(site));
 			return -EINVAL;
 		}
 
-		site->key = ((long)tramp_key->key - (long)&tramp_key->key) |
+		site->key = (key - (long)&site->key) |
 			    (site->key & STATIC_CALL_SITE_FLAGS);
 	}