[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YYrCn3mc+EbY+OB/@hirez.programming.kicks-ass.net>
Date: Tue, 9 Nov 2021 19:49:03 +0100
From: Peter Zijlstra <peterz@...radead.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
Josh Poimboeuf <jpoimboe@...hat.com>,
Jason Baron <jbaron@...mai.com>,
Steven Rostedt <rostedt@...dmis.org>,
Mark Rutland <mark.rutland@....com>,
Kees Cook <keescook@...omium.org>
Subject: Re: [RFC PATCH 2/7] static_call: deal with unexported keys without
cluttering up the API
On Tue, Nov 09, 2021 at 05:45:44PM +0100, Ard Biesheuvel wrote:
> diff --git a/include/linux/static_call_types.h b/include/linux/static_call_types.h
> index 5a00b8b2cf9f..0bb36294cce7 100644
> --- a/include/linux/static_call_types.h
> +++ b/include/linux/static_call_types.h
> @@ -32,15 +32,20 @@
> struct static_call_site {
> s32 addr;
> s32 key;
> + s32 tramp;
> };
I can't say I'm thrilled at growing this thing, but the cleanup is nice.
Perhaps we can increase alignment on struct static_call_key and instead
frob it in .key still?
>
> #define DECLARE_STATIC_CALL(name, func) \
> - extern struct static_call_key STATIC_CALL_KEY(name); \
> + extern __weak struct static_call_key STATIC_CALL_KEY(name); \
> extern typeof(func) STATIC_CALL_TRAMP(name);
I'm a little bit confused on how this actually works. What does a __weak
extern data symbol do?
A __weak function definition would create a module local instance of the
function barring a strong override.
But what does a __weak extern do?
> diff --git a/kernel/static_call.c b/kernel/static_call.c
> index 43ba0b1e0edb..360cc3cd0fbf 100644
> --- a/kernel/static_call.c
> +++ b/kernel/static_call.c
> @@ -366,18 +366,18 @@ static int static_call_add_module(struct module *mod)
> * means modules are allowed to call static_call_update() on
> * it.
> *
> - * Otherwise, the key isn't exported, and 'addr' points to the
> + * Otherwise, the key isn't exported, and 'tramp' 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))
> + if (addr)
> continue;
This seems to imply that the __weak extern symbol gets resolved to 0 at
module link time.
>
> - key = tramp_key_lookup(addr);
> + key = tramp_key_lookup((unsigned long)offset_to_ptr(&site->tramp));
> if (!key) {
> - pr_warn("Failed to fixup __raw_static_call() usage at: %ps\n",
> + pr_warn("Failed to fixup static_call() usage at: %ps\n",
> static_call_addr(site));
> return -EINVAL;
> }
Powered by blists - more mailing lists