From 92e2c5feecb39cdd4796da89f2b684e395403040 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 8 Jan 2024 13:31:04 -0500 Subject: [PATCH] Fix: Disable IBT around indirect function calls When the Intel IBT feature is enabled, a CPU supporting this feature validates that all indirect jumps/calls land on an ENDBR64 instruction. The kernel seals functions which are not meant to be called indirectly, which means that calling functions indirectly from their address fetched using kallsyms or kprobes trigger a crash. Use the MSR_IA32_S_CET CET_ENDBR_EN MSR bit to temporarily disable ENDBR validation around indirect calls to kernel functions. Considering that the main purpose of this feature is to prevent ROP-style attacks, disabling the ENDBR validation temporarily around the call from a kernel module does not affect the ROP protection. Fixes #1408 Signed-off-by: Mathieu Desnoyers Change-Id: I97f5d8efce093c1e956cede1f44de2fcebf30227 --- include/wrapper/ibt.h | 59 ++++++++++++++++++++ include/wrapper/kallsyms.h | 2 + src/lttng-context-callstack-legacy-impl.h | 3 + src/lttng-context-callstack-stackwalk-impl.h | 5 ++ src/wrapper/irqdesc.c | 8 ++- src/wrapper/kallsyms.c | 12 +++- src/wrapper/page_alloc.c | 8 ++- 7 files changed, 92 insertions(+), 5 deletions(-) create mode 100644 include/wrapper/ibt.h diff --git a/include/wrapper/ibt.h b/include/wrapper/ibt.h new file mode 100644 index 00000000..e089a8f6 --- /dev/null +++ b/include/wrapper/ibt.h @@ -0,0 +1,59 @@ +/* SPDX-License-Identifier: (GPL-2.0-only) + * + * wrapper/ibt.h + * + * Copyright (C) 2024 Mathieu Desnoyers + */ + +#ifndef _LTTNG_WRAPPER_IBT_H +#define _LTTNG_WRAPPER_IBT_H + +struct irq_ibt_state { + u64 msr; + unsigned long flags; +}; + +/* + * Save (disable) and restore interrupts around MSR bit change and indirect + * function call to make sure this thread is not migrated to another CPU which + * would not have the MSR bit cleared. + */ + +#ifdef CONFIG_X86_KERNEL_IBT +# include +# include +static inline __attribute__((always_inline)) +struct irq_ibt_state wrapper_irq_ibt_save(void) +{ + struct irq_ibt_state state = { 0, 0 }; + u64 msr; + + if (!cpu_feature_enabled(X86_FEATURE_IBT)) + goto end; + local_irq_save(state.flags); + rdmsrl(MSR_IA32_S_CET, msr); + wrmsrl(MSR_IA32_S_CET, msr & ~CET_ENDBR_EN); + state.msr = msr; +end: + return state; +} + +static inline __attribute__((always_inline)) +void wrapper_irq_ibt_restore(struct irq_ibt_state state) +{ + u64 msr; + + if (!cpu_feature_enabled(X86_FEATURE_IBT)) + return; + rdmsrl(MSR_IA32_S_CET, msr); + msr &= ~CET_ENDBR_EN; + msr |= (state.msr & CET_ENDBR_EN); + wrmsrl(MSR_IA32_S_CET, msr); + local_irq_restore(state.flags); +} +#else +static inline struct irq_ibt_state wrapper_irq_ibt_save(void) { struct irq_ibt_state state = { 0, 0 }; return state; } +static inline void wrapper_irq_ibt_restore(struct irq_ibt_state state) { } +#endif + +#endif /* _LTTNG_WRAPPER_IBT_H */ diff --git a/include/wrapper/kallsyms.h b/include/wrapper/kallsyms.h index fe2daefb..8129833f 100644 --- a/include/wrapper/kallsyms.h +++ b/include/wrapper/kallsyms.h @@ -16,6 +16,8 @@ #include #include +#include + /* CONFIG_PPC64_ELF_ABI_V1/V2 were introduced in v5.19 */ #if defined(CONFIG_PPC64_ELF_ABI_V2) || (defined(CONFIG_PPC64) && defined(CONFIG_CPU_LITTLE_ENDIAN)) #define LTTNG_CONFIG_PPC64_ELF_ABI_V2 diff --git a/src/lttng-context-callstack-legacy-impl.h b/src/lttng-context-callstack-legacy-impl.h index 833443d6..ba176de9 100644 --- a/src/lttng-context-callstack-legacy-impl.h +++ b/src/lttng-context-callstack-legacy-impl.h @@ -150,6 +150,7 @@ size_t lttng_callstack_sequence_get_size(void *priv, struct lttng_kernel_probe_c struct field_data *fdata = (struct field_data *) priv; size_t orig_offset = offset; int cpu = smp_processor_id(); + struct irq_ibt_state irq_ibt_state; /* do not write data if no space is available */ trace = stack_trace_context(fdata, cpu); @@ -165,7 +166,9 @@ size_t lttng_callstack_sequence_get_size(void *priv, struct lttng_kernel_probe_c ++per_cpu(callstack_user_nesting, cpu); /* do the real work and reserve space */ + irq_ibt_state = wrapper_irq_ibt_save(); cs_types[fdata->mode].save_func(trace); + wrapper_irq_ibt_restore(irq_ibt_state); if (fdata->mode == CALLSTACK_USER) per_cpu(callstack_user_nesting, cpu)--; diff --git a/src/lttng-context-callstack-stackwalk-impl.h b/src/lttng-context-callstack-stackwalk-impl.h index e73d1156..5075e985 100644 --- a/src/lttng-context-callstack-stackwalk-impl.h +++ b/src/lttng-context-callstack-stackwalk-impl.h @@ -152,6 +152,7 @@ size_t lttng_callstack_sequence_get_size(void *priv, struct lttng_kernel_probe_c struct field_data *fdata = (struct field_data *) priv; size_t orig_offset = offset; int cpu = smp_processor_id(); + struct irq_ibt_state irq_ibt_state; /* do not write data if no space is available */ trace = stack_trace_context(fdata, cpu); @@ -166,14 +167,18 @@ size_t lttng_callstack_sequence_get_size(void *priv, struct lttng_kernel_probe_c switch (fdata->mode) { case CALLSTACK_KERNEL: /* do the real work and reserve space */ + irq_ibt_state = wrapper_irq_ibt_save(); trace->nr_entries = save_func_kernel(trace->entries, MAX_ENTRIES, 0); + wrapper_irq_ibt_restore(irq_ibt_state); break; case CALLSTACK_USER: ++per_cpu(callstack_user_nesting, cpu); /* do the real work and reserve space */ + irq_ibt_state = wrapper_irq_ibt_save(); trace->nr_entries = save_func_user(trace->entries, MAX_ENTRIES); + wrapper_irq_ibt_restore(irq_ibt_state); per_cpu(callstack_user_nesting, cpu)--; break; default: diff --git a/src/wrapper/irqdesc.c b/src/wrapper/irqdesc.c index 1bfae91a..b77b044b 100644 --- a/src/wrapper/irqdesc.c +++ b/src/wrapper/irqdesc.c @@ -29,7 +29,13 @@ struct irq_desc *wrapper_irq_to_desc(unsigned int irq) if (!irq_to_desc_sym) irq_to_desc_sym = (void *) kallsyms_lookup_funcptr("irq_to_desc"); if (irq_to_desc_sym) { - return irq_to_desc_sym(irq); + struct irq_ibt_state irq_ibt_state; + struct irq_desc *ret; + + irq_ibt_state = wrapper_irq_ibt_save(); + ret = irq_to_desc_sym(irq); + wrapper_irq_ibt_restore(irq_ibt_state); + return ret; } else { printk_once(KERN_WARNING "LTTng: irq_to_desc symbol lookup failed.\n"); return NULL; diff --git a/src/wrapper/kallsyms.c b/src/wrapper/kallsyms.c index 2535c2a5..295617f6 100644 --- a/src/wrapper/kallsyms.c +++ b/src/wrapper/kallsyms.c @@ -127,9 +127,15 @@ unsigned long wrapper_kallsyms_lookup_name(const char *name) if (!kallsyms_lookup_name_sym) { kallsyms_lookup_name_sym = (void *)do_get_kallsyms(); } - if (kallsyms_lookup_name_sym) - return kallsyms_lookup_name_sym(name); - else { + if (kallsyms_lookup_name_sym) { + struct irq_ibt_state irq_ibt_state; + unsigned long ret; + + irq_ibt_state = wrapper_irq_ibt_save(); + ret = kallsyms_lookup_name_sym(name); + wrapper_irq_ibt_restore(irq_ibt_state); + return ret; + } else { printk_once(KERN_WARNING "LTTng: requires kallsyms_lookup_name\n"); return 0; } diff --git a/src/wrapper/page_alloc.c b/src/wrapper/page_alloc.c index a03838fa..5e19d7b5 100644 --- a/src/wrapper/page_alloc.c +++ b/src/wrapper/page_alloc.c @@ -30,7 +30,13 @@ unsigned long wrapper_get_pfnblock_flags_mask(struct page *page, { WARN_ON_ONCE(!get_pfnblock_flags_mask_sym); if (get_pfnblock_flags_mask_sym) { - return get_pfnblock_flags_mask_sym(page, pfn, end_bitidx, mask); + struct irq_ibt_state irq_ibt_state; + unsigned long ret; + + irq_ibt_state = wrapper_irq_ibt_save(); + ret = get_pfnblock_flags_mask_sym(page, pfn, end_bitidx, mask); + wrapper_irq_ibt_restore(irq_ibt_state); + return ret; } else { return -ENOSYS; } -- 2.34.1