Messages in this thread | | | Date | Tue, 9 Nov 2021 19:49:03 +0100 | From | Peter Zijlstra <> | 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;
> }
| |