Messages in this thread | | | From | Ard Biesheuvel <> | Date | Fri, 22 Jan 2021 17:57:53 +0100 | Subject | Re: [RFC PATCH 6/8] preempt/dynamic: Provide preempt_schedule[_notrace]() static calls |
| |
On Fri, 22 Jan 2021 at 17:53, Peter Zijlstra <peterz@infradead.org> wrote: > > On Mon, Jan 18, 2021 at 03:12:21PM +0100, Frederic Weisbecker wrote: > > +#ifdef CONFIG_PREEMPT_DYNAMIC > > +DEFINE_STATIC_CALL(preempt_schedule, __preempt_schedule_func()); > > +EXPORT_STATIC_CALL(preempt_schedule); > > +#endif > > > +#ifdef CONFIG_PREEMPT_DYNAMIC > > +DEFINE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func()); > > +EXPORT_STATIC_CALL(preempt_schedule_notrace); > > +#endif > > So one of the things I hates most of this is that is allows 'random' > modules to hijack the preemption by rewriting these callsites. Once you > export the key, we've lost. >
Are these supposed to be switchable at any time? Or only at boot? In the latter case, can't we drop the associated data structure in __ro_after_init so it becomes R/O when booting completes?
> I've tried a number of things, but this is the only one I could come up > with that actually stands a chance against malicious modules (vbox and > the like). > > It's somewhat elaborate, but afaict it actually works. > > --- > > --- a/arch/x86/include/asm/preempt.h > +++ b/arch/x86/include/asm/preempt.h > @@ -114,7 +114,7 @@ DECLARE_STATIC_CALL(preempt_schedule, __ > > #define __preempt_schedule() \ > do { \ > - __ADDRESSABLE(STATIC_CALL_KEY(preempt_schedule)); \ > + __STATIC_CALL_MOD_ADDRESSABLE(preempt_schedule); \ > asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule) : ASM_CALL_CONSTRAINT); \ > } while (0) > > @@ -127,7 +127,7 @@ DECLARE_STATIC_CALL(preempt_schedule_not > > #define __preempt_schedule_notrace() \ > do { \ > - __ADDRESSABLE(STATIC_CALL_KEY(preempt_schedule_notrace)); \ > + __STATIC_CALL_MOD_ADDRESSABLE(preempt_schedule_notrace); \ > asm volatile ("call " STATIC_CALL_TRAMP_STR(preempt_schedule_notrace) : ASM_CALL_CONSTRAINT); \ > } while (0) > > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -93,7 +93,7 @@ DECLARE_STATIC_CALL(might_resched, __con > > static __always_inline void might_resched(void) > { > - static_call(might_resched)(); > + static_call_mod(might_resched)(); > } > > #else > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1880,7 +1880,7 @@ DECLARE_STATIC_CALL(cond_resched, __cond > > static __always_inline int _cond_resched(void) > { > - return static_call(cond_resched)(); > + return static_call_mod(cond_resched)(); > } > > #else > --- a/include/linux/static_call.h > +++ b/include/linux/static_call.h > @@ -107,6 +107,10 @@ extern void arch_static_call_transform(v > > #define STATIC_CALL_TRAMP_ADDR(name) &STATIC_CALL_TRAMP(name) > > +#define static_call_register(name) \ > + __static_call_register(&STATIC_CALL_KEY(name), \ > + &STATIC_CALL_TRAMP(name)) > + > #else > #define STATIC_CALL_TRAMP_ADDR(name) NULL > #endif > @@ -138,6 +142,7 @@ struct static_call_key { > }; > }; > > +extern int __static_call_register(struct static_call_key *key, void *tramp); > extern void __static_call_update(struct static_call_key *key, void *tramp, void *func); > extern int static_call_mod_init(struct module *mod); > extern int static_call_text_reserved(void *start, void *end); > @@ -162,6 +167,9 @@ extern long __static_call_return0(void); > > #define static_call_cond(name) (void)__static_call(name) > > +#define EXPORT_STATIC_CALL_TRAMP(name) \ > + EXPORT_SYMBOL(STATIC_CALL_TRAMP(name)) > + > #define EXPORT_STATIC_CALL(name) \ > EXPORT_SYMBOL(STATIC_CALL_KEY(name)); \ > EXPORT_SYMBOL(STATIC_CALL_TRAMP(name)) > @@ -194,6 +202,11 @@ struct static_call_key { > > #define static_call_cond(name) (void)__static_call(name) > > +static inline int __static_call_register(struct static_call_key *key, void *tramp) > +{ > + return 0; > +} > + > static inline > void __static_call_update(struct static_call_key *key, void *tramp, void *func) > { > @@ -213,6 +226,9 @@ static inline long __static_call_return0 > return 0; > } > > +#define EXPORT_STATIC_CALL_TRAMP(name) \ > + EXPORT_SYMBOL(STATIC_CALL_TRAMP(name)) > + > #define EXPORT_STATIC_CALL(name) \ > EXPORT_SYMBOL(STATIC_CALL_KEY(name)); \ > EXPORT_SYMBOL(STATIC_CALL_TRAMP(name)) > --- a/include/linux/static_call_types.h > +++ b/include/linux/static_call_types.h > @@ -39,17 +39,39 @@ struct static_call_site { > > #ifdef CONFIG_HAVE_STATIC_CALL > > +#define __raw_static_call(name) (&STATIC_CALL_TRAMP(name)) > + > +#ifdef CONFIG_HAVE_STATIC_CALL_INLINE > + > /* > * __ADDRESSABLE() is used to ensure the key symbol doesn't get stripped from > * the symbol table so that objtool can reference it when it generates the > * .static_call_sites section. > */ > +#define __STATIC_CALL_ADDRESSABLE(name) \ > + __ADDRESSABLE(STATIC_CALL_KEY(name)) > + > #define __static_call(name) \ > ({ \ > - __ADDRESSABLE(STATIC_CALL_KEY(name)); \ > - &STATIC_CALL_TRAMP(name); \ > + __STATIC_CALL_ADDRESSABLE(name); \ > + __raw_static_call(name); \ > }) > > +#else /* !CONFIG_HAVE_STATIC_CALL_INLINE */ > + > +#define __STATIC_CALL_ADDRESSABLE(name) > +#define __static_call(name) __raw_static_call(name) > + > +#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 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -5268,7 +5268,7 @@ EXPORT_SYMBOL(preempt_schedule); > > #ifdef CONFIG_PREEMPT_DYNAMIC > DEFINE_STATIC_CALL(preempt_schedule, __preempt_schedule_func()); > -EXPORT_STATIC_CALL(preempt_schedule); > +EXPORT_STATIC_CALL_TRAMP(preempt_schedule); > #endif > > > @@ -5326,7 +5326,7 @@ EXPORT_SYMBOL_GPL(preempt_schedule_notra > > #ifdef CONFIG_PREEMPT_DYNAMIC > DEFINE_STATIC_CALL(preempt_schedule_notrace, __preempt_schedule_notrace_func()); > -EXPORT_STATIC_CALL(preempt_schedule_notrace); > +EXPORT_STATIC_CALL_TRAMP(preempt_schedule_notrace); > #endif > > #endif /* CONFIG_PREEMPTION */ > @@ -6879,10 +6879,10 @@ EXPORT_SYMBOL(__cond_resched); > > #ifdef CONFIG_PREEMPT_DYNAMIC > DEFINE_STATIC_CALL_RET0(cond_resched, __cond_resched); > -EXPORT_STATIC_CALL(cond_resched); > +EXPORT_STATIC_CALL_TRAMP(cond_resched); > > DEFINE_STATIC_CALL_RET0(might_resched, __cond_resched); > -EXPORT_STATIC_CALL(might_resched); > +EXPORT_STATIC_CALL_TRAMP(might_resched); > #endif > > /* > @@ -8096,6 +8096,13 @@ void __init sched_init(void) > > init_uclamp(); > > +#ifdef CONFIG_PREEMPT_DYNAMIC > + static_call_register(cond_resched); > + static_call_register(might_resched); > + static_call_register(preempt_schedule); > + static_call_register(preempt_schedule_notrace); > +#endif > + > scheduler_running = 1; > } > > --- a/kernel/static_call.c > +++ b/kernel/static_call.c > @@ -323,10 +323,85 @@ static int __static_call_mod_text_reserv > return ret; > } > > +struct static_call_ass { > + struct rb_node node; > + struct static_call_key *key; > + unsigned long tramp; > +}; > + > +static struct rb_root static_call_asses; > + > +#define __node_2_ass(_n) \ > + rb_entry((_n), struct static_call_ass, node) > + > +static inline bool ass_less(struct rb_node *a, const struct rb_node *b) > +{ > + return __node_2_ass(a)->tramp < __node_2_ass(b)->tramp; > +} > + > +static inline int ass_cmp(const void *a, const struct rb_node *b) > +{ > + if (*(unsigned long *)a < __node_2_ass(b)->tramp) > + return -1; > + > + if (*(unsigned long *)a > __node_2_ass(b)->tramp) > + return 1; > + > + return 0; > +} > + > +int __static_call_register(struct static_call_key *key, void *tramp) > +{ > + struct static_call_ass *ass = kzalloc(sizeof(*ass), GFP_KERNEL); > + if (!ass) > + return -ENOMEM; > + > + ass->key = key; > + ass->tramp = (unsigned long)tramp; > + > + /* trampolines should be aligned */ > + WARN_ON_ONCE(ass->tramp & STATIC_CALL_SITE_FLAGS); > + > + rb_add(&ass->node, &static_call_asses, &ass_less); > + return 0; > +} > + > +static struct static_call_ass *static_call_find_ass(unsigned long addr) > +{ > + struct rb_node *node = rb_find(&addr, &static_call_asses, &ass_cmp); > + if (!node) > + return NULL; > + return __node_2_ass(node); > +} > + > 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_ass *ass; > + > + /* > + * Gotta fix up the keys that point to the trampoline. > + */ > + if (!kernel_text_address(addr)) > + continue; > + > + ass = static_call_find_ass(addr); > + if (!ass) { > + pr_warn("Failed to fixup __raw_static_call() usage at: %ps\n", > + static_call_addr(site)); > + return -EINVAL; > + } > + > + site->key = ((unsigned long)ass->key - (unsigned long)&site->key) | > + (site->key & STATIC_CALL_SITE_FLAGS); > + } > + > + return __static_call_init(mod, start, stop); > } > > static void static_call_del_module(struct module *mod) > @@ -392,6 +467,11 @@ static struct notifier_block static_call > > #else > > +int __static_call_register(struct static_call_key *key, void *tramp) > +{ > + return 0; > +} > + > static inline int __static_call_mod_text_reserved(void *start, void *end) > { > return 0; > --- a/tools/include/linux/static_call_types.h > +++ b/tools/include/linux/static_call_types.h > @@ -39,17 +39,39 @@ struct static_call_site { > > #ifdef CONFIG_HAVE_STATIC_CALL > > +#define __raw_static_call(name) (&STATIC_CALL_TRAMP(name)) > + > +#ifdef CONFIG_HAVE_STATIC_CALL_INLINE > + > /* > * __ADDRESSABLE() is used to ensure the key symbol doesn't get stripped from > * the symbol table so that objtool can reference it when it generates the > * .static_call_sites section. > */ > +#define __STATIC_CALL_ADDRESSABLE(name) \ > + __ADDRESSABLE(STATIC_CALL_KEY(name)) > + > #define __static_call(name) \ > ({ \ > - __ADDRESSABLE(STATIC_CALL_KEY(name)); \ > - &STATIC_CALL_TRAMP(name); \ > + __STATIC_CALL_ADDRESSABLE(name); \ > + __raw_static_call(name); \ > }) > > +#else /* !CONFIG_HAVE_STATIC_CALL_INLINE */ > + > +#define __STATIC_CALL_ADDRESSABLE(name) > +#define __static_call(name) __raw_static_call(name) > + > +#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 > --- a/tools/objtool/check.c > +++ b/tools/objtool/check.c > @@ -502,8 +502,16 @@ static int create_static_call_sections(s > > key_sym = find_symbol_by_name(file->elf, tmp); > if (!key_sym) { > - WARN("static_call: can't find static_call_key symbol: %s", tmp); > - return -1; > + if (!module) { > + WARN("static_call: can't find static_call_key symbol: %s", tmp); > + return -1; > + } > + /* > + * For static_call_mod() we allow the key to be the > + * trampoline address. This is fixed up in > + * static_call_add_module(). > + */ > + key_sym = insn->call_dest; > } > free(key_name); >
| |