Messages in this thread Patch in this message | | | From | Ard Biesheuvel <> | Subject | [RFC PATCH 2/7] static_call: deal with unexported keys without cluttering up the API | Date | Tue, 9 Nov 2021 17:45:44 +0100 |
| |
In principle, invoking a static call does not require access to the static call key that is used to manipulate the function that the static call targets. This means that exporting only the static call but not its key to modules should prevent such modules from manipulating the target inadvertently.
However, for inline static calls, we do need access to the key at module load time, because the code patching routines rely on it. For this reason, there is special handling in objtool that recovers the key pointer from the address of the trampoline (which does get exported unconditionally), using a lookup table in the core kernel.
Unfortunately, this clutters up the user-visible API: a static call is normally invoked using the static_call() macro, but in cases where a) the call originates from a module and b) the associated key was not exported, it is left up to the user to pick the static_call_mod() macro instead. Since that macro relies on the lookup table in the core kernel, it can only be used on static calls that are exported from the core kernel.
It would be much better if the call sites can simply use static_call() and not have to figure out whether or not the static call exports its key and/or whether it as exported from the core kernel.
So let's address this, by using a weak reference for the key. This means we can force the symbol reference in the module to be emitted unconditionally, allowing the caller to use static_call() without knowing the difference. This does imply that the logic in objtool that tests whether a symbol exists in the object no longer works, as the symbol is guaranteed to exist. So instead, add another field to struct static_call_site to cover the trampoline reference, and fix up the module notifier that consumes this data accordingly.
This gets rid of the following macros:
__STATIC_CALL_MOD_ADDRESSABLE() static_call_mod() __static_call() __raw_static_call()
Signed-off-by: Ard Biesheuvel <ardb@kernel.org> --- arch/x86/include/asm/preempt.h | 4 +-- include/linux/kernel.h | 2 +- include/linux/sched.h | 2 +- include/linux/static_call_types.h | 26 +++++----------- kernel/static_call.c | 8 ++--- tools/include/linux/static_call_types.h | 26 +++++----------- tools/objtool/check.c | 31 ++++++++++---------- 7 files changed, 38 insertions(+), 61 deletions(-)
diff --git a/arch/x86/include/asm/preempt.h b/arch/x86/include/asm/preempt.h index fe5efbcba824..341745a03726 100644 --- a/arch/x86/include/asm/preempt.h +++ b/arch/x86/include/asm/preempt.h @@ -121,7 +121,7 @@ DECLARE_STATIC_CALL(preempt_schedule, __preempt_schedule_func); #define __preempt_schedule() \ do { \ - __STATIC_CALL_MOD_ADDRESSABLE(preempt_schedule); \ + __STATIC_CALL_ADDRESSABLE(preempt_schedule); \ asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule) : ASM_CALL_CONSTRAINT); \ } while (0) @@ -129,7 +129,7 @@ DECLARE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func); #define __preempt_schedule_notrace() \ do { \ - __STATIC_CALL_MOD_ADDRESSABLE(preempt_schedule_notrace); \ + __STATIC_CALL_ADDRESSABLE(preempt_schedule_notrace); \ asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule_notrace) : ASM_CALL_CONSTRAINT); \ } while (0) diff --git a/include/linux/kernel.h b/include/linux/kernel.h index e5359b09de1d..1e1159878e40 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -101,7 +101,7 @@ DECLARE_STATIC_CALL(might_resched, __cond_resched); static __always_inline void might_resched(void) { - static_call_mod(might_resched)(); + static_call(might_resched)(); } #else diff --git a/include/linux/sched.h b/include/linux/sched.h index 78c351e35fec..3cdf55c1074c 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -2014,7 +2014,7 @@ DECLARE_STATIC_CALL(cond_resched, __cond_resched); static __always_inline int _cond_resched(void) { - return static_call_mod(cond_resched)(); + return static_call(cond_resched)(); } #else 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; }; #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); #ifdef CONFIG_HAVE_STATIC_CALL -#define __raw_static_call(name) (&STATIC_CALL_TRAMP(name)) +#define static_call(name) \ +({ \ + __STATIC_CALL_ADDRESSABLE(name); \ + (&STATIC_CALL_TRAMP(name)); \ +}) #ifdef CONFIG_HAVE_STATIC_CALL_INLINE @@ -52,12 +57,6 @@ struct static_call_site { #define __STATIC_CALL_ADDRESSABLE(name) \ __ADDRESSABLE(STATIC_CALL_KEY(name)) -#define __static_call(name) \ -({ \ - __STATIC_CALL_ADDRESSABLE(name); \ - __raw_static_call(name); \ -}) - struct static_call_key { void *func; union { @@ -71,7 +70,6 @@ struct static_call_key { #else /* !CONFIG_HAVE_STATIC_CALL_INLINE */ #define __STATIC_CALL_ADDRESSABLE(name) -#define __static_call(name) __raw_static_call(name) struct static_call_key { void *func; @@ -79,16 +77,6 @@ struct static_call_key { #endif /* CONFIG_HAVE_STATIC_CALL_INLINE */ -#ifdef MODULE -#define __STATIC_CALL_MOD_ADDRESSABLE(name) -#define static_call_mod(name) __raw_static_call(name) -#else -#define __STATIC_CALL_MOD_ADDRESSABLE(name) __STATIC_CALL_ADDRESSABLE(name) -#define static_call_mod(name) __static_call(name) -#endif - -#define static_call(name) __static_call(name) - #else struct static_call_key { 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; - 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; } diff --git a/tools/include/linux/static_call_types.h b/tools/include/linux/static_call_types.h index 5a00b8b2cf9f..0bb36294cce7 100644 --- a/tools/include/linux/static_call_types.h +++ b/tools/include/linux/static_call_types.h @@ -32,15 +32,20 @@ struct static_call_site { s32 addr; s32 key; + s32 tramp; }; #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); #ifdef CONFIG_HAVE_STATIC_CALL -#define __raw_static_call(name) (&STATIC_CALL_TRAMP(name)) +#define static_call(name) \ +({ \ + __STATIC_CALL_ADDRESSABLE(name); \ + (&STATIC_CALL_TRAMP(name)); \ +}) #ifdef CONFIG_HAVE_STATIC_CALL_INLINE @@ -52,12 +57,6 @@ struct static_call_site { #define __STATIC_CALL_ADDRESSABLE(name) \ __ADDRESSABLE(STATIC_CALL_KEY(name)) -#define __static_call(name) \ -({ \ - __STATIC_CALL_ADDRESSABLE(name); \ - __raw_static_call(name); \ -}) - struct static_call_key { void *func; union { @@ -71,7 +70,6 @@ struct static_call_key { #else /* !CONFIG_HAVE_STATIC_CALL_INLINE */ #define __STATIC_CALL_ADDRESSABLE(name) -#define __static_call(name) __raw_static_call(name) struct static_call_key { void *func; @@ -79,16 +77,6 @@ struct static_call_key { #endif /* CONFIG_HAVE_STATIC_CALL_INLINE */ -#ifdef MODULE -#define __STATIC_CALL_MOD_ADDRESSABLE(name) -#define static_call_mod(name) __raw_static_call(name) -#else -#define __STATIC_CALL_MOD_ADDRESSABLE(name) __STATIC_CALL_ADDRESSABLE(name) -#define static_call_mod(name) __static_call(name) -#endif - -#define static_call(name) __static_call(name) - #else struct static_call_key { diff --git a/tools/objtool/check.c b/tools/objtool/check.c index add39902166d..ec6ddaaf6cf6 100644 --- a/tools/objtool/check.c +++ b/tools/objtool/check.c @@ -652,21 +652,8 @@ static int create_static_call_sections(struct objtool_file *file) key_sym = find_symbol_by_name(file->elf, tmp); if (!key_sym) { - if (!module) { - WARN("static_call: can't find static_call_key symbol: %s", tmp); - return -1; - } - - /* - * For modules(), the key might not be exported, which - * means the module can make static calls but isn't - * allowed to change them. - * - * In that case we temporarily set the key to be the - * trampoline address. This is fixed up in - * static_call_add_module(). - */ - key_sym = insn->call_dest; + WARN("static_call: can't find static_call_key symbol: %s", tmp); + return -1; } free(key_name); @@ -677,6 +664,20 @@ static int create_static_call_sections(struct objtool_file *file) is_sibling_call(insn) * STATIC_CALL_SITE_TAIL)) return -1; + /* + * For modules(), the key might not be exported, which means + * the module can make static calls but isn't allowed to change + * them. + * + * For this case, we pass the trampoline in .static_call_sites + * as well. This is used to lookup the key in + * static_call_add_module(). + */ + if (elf_add_reloc(file->elf, sec, + idx * sizeof(struct static_call_site) + 8, + R_X86_64_PC32, insn->call_dest, 0)) + return -1; + idx++; } -- 2.30.2
| |