Messages in this thread Patch in this message | | | From | Ard Biesheuvel <> | Date | Tue, 9 Nov 2021 19:53:33 +0100 | Subject | Re: [RFC PATCH 2/7] static_call: deal with unexported keys without cluttering up the API |
| |
On Tue, 9 Nov 2021 at 19:49, Peter Zijlstra <peterz@infradead.org> wrote: > > 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? >
This is already a place-relative field, and one points into the data section and the other into text. So I don't see how we can squeeze enough bits out of it to make this fit.
> > > > #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? >
It is simply a reference that is permitted to remain undefined.
That is why (in another patch) I do sth like
extern __weak foo;
if (&foo) ... use foo else ... use sth else
> > 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. >
Yes, if the referenced symbol is not exported, it just uses 0x0 for its value.
> > > > - 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; > > > }
| |