[v2,59/70] x86/bugframe: CFI hardening

Commit Message

Andrew Cooper Feb. 14, 2022, 12:51 p.m. UTC

Control Flow Integrity schemes use toolchain and optionally hardware support
to help protect against call/jump/return oriented programming attacks.

Use cf_check to annotate function pointer targets for the toolchain.

run_in_exception_handler() managed to escape typechecking, as the compiler
can't see where function pointer gets called.  After adding some ad-hoc
typechecking, it turns out that dump_execution_state() alone differs in
const-ness from the other users of run_in_exception_handler().

Introduce a new show_execution_state_nonconst() to make the typechecking
happy.

Signed-off-by: Andrew Cooper <[e-mail]>
Acked-by: Jan Beulich <[e-mail]>
---
 xen/arch/x86/include/asm/bug.h       | 10 +++++++++-
 xen/arch/x86/include/asm/processor.h |  4 +++-
 xen/arch/x86/traps.c                 |  5 +++++
 xen/common/keyhandler.c              |  4 ++--
 xen/drivers/char/ehci-dbgp.c         |  2 +-
 xen/drivers/char/ns16550.c           |  2 +-
 xen/include/xen/lib.h                |  2 +-
 7 files changed, 22 insertions(+), 7 deletions(-)

Patch

diff --git a/xen/arch/x86/include/asm/bug.h b/xen/arch/x86/include/asm/bug.h
index 9bb4a194202f..b7265bdfbe33 100644
--- a/xen/arch/x86/include/asm/bug.h
+++ b/xen/arch/x86/include/asm/bug.h
@@ -65,7 +65,15 @@  struct bug_frame {
     unreachable();                                              \
 } while (0)
 
-#define run_in_exception_handler(fn) BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL)
+/*
+ * TODO: untangle header dependences, break BUILD_BUG_ON() out of xen/lib.h,
+ * and use a real static inline here to get proper type checking of fn().
+ */
+#define run_in_exception_handler(fn)                            \
+    do {                                                        \
+        (void)((fn) == (void (*)(struct cpu_user_regs *))NULL); \
+        BUG_FRAME(BUGFRAME_run_fn, 0, fn, 0, NULL);             \
+    } while ( 0 )
 
 #define assert_failed(msg) do {                                 \
     BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg);     \
diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index 23639d5479a3..8e2816fae9b9 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -496,7 +496,9 @@  void show_code(const struct cpu_user_regs *regs);
 void show_stack_overflow(unsigned int cpu, const struct cpu_user_regs *regs);
 void show_registers(const struct cpu_user_regs *regs);
 void show_execution_state(const struct cpu_user_regs *regs);
-#define dump_execution_state() run_in_exception_handler(show_execution_state)
+void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs);
+#define dump_execution_state() \
+    run_in_exception_handler(show_execution_state_nonconst)
 void show_page_walk(unsigned long addr);
 void noreturn fatal_trap(const struct cpu_user_regs *regs, bool_t show_remote);
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 7b957101934e..a2278d9499d0 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -681,6 +681,11 @@  void show_execution_state(const struct cpu_user_regs *regs)
     console_unlock_recursive_irqrestore(flags);
 }
 
+void cf_check show_execution_state_nonconst(struct cpu_user_regs *regs)
+{
+    show_execution_state(regs);
+}
+
 void vcpu_show_execution_state(struct vcpu *v)
 {
     unsigned long flags = 0;
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index 5dc650a37c5c..b6e22d8120b1 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -138,7 +138,7 @@  static void cf_check show_handlers(unsigned char key)
 
 static cpumask_t dump_execstate_mask;
 
-void dump_execstate(struct cpu_user_regs *regs)
+void cf_check dump_execstate(struct cpu_user_regs *regs)
 {
     unsigned int cpu = smp_processor_id();
 
@@ -490,7 +490,7 @@  static void cf_check run_all_keyhandlers(
     tasklet_schedule(&run_all_keyhandlers_tasklet);
 }
 
-static void do_debugger_trap_fatal(struct cpu_user_regs *regs)
+static void cf_check do_debugger_trap_fatal(struct cpu_user_regs *regs)
 {
     (void)debugger_trap_fatal(0xf001, regs);
 
diff --git a/xen/drivers/char/ehci-dbgp.c b/xen/drivers/char/ehci-dbgp.c
index e205c0da6a61..16c8ff394d5c 100644
--- a/xen/drivers/char/ehci-dbgp.c
+++ b/xen/drivers/char/ehci-dbgp.c
@@ -1247,7 +1247,7 @@  static int cf_check ehci_dbgp_getc(struct serial_port *port, char *pc)
 /* Safe: ehci_dbgp_poll() runs as timer handler, so not reentrant. */
 static struct serial_port *poll_port;
 
-static void _ehci_dbgp_poll(struct cpu_user_regs *regs)
+static void cf_check _ehci_dbgp_poll(struct cpu_user_regs *regs)
 {
     struct serial_port *port = poll_port;
     struct ehci_dbgp *dbgp = port->uart;
diff --git a/xen/drivers/char/ns16550.c b/xen/drivers/char/ns16550.c
index 8df1ee4d5c2c..e5b4a9085516 100644
--- a/xen/drivers/char/ns16550.c
+++ b/xen/drivers/char/ns16550.c
@@ -206,7 +206,7 @@  static void cf_check ns16550_interrupt(
 /* Safe: ns16550_poll() runs as softirq so not reentrant on a given CPU. */
 static DEFINE_PER_CPU(struct serial_port *, poll_port);
 
-static void __ns16550_poll(struct cpu_user_regs *regs)
+static void cf_check __ns16550_poll(struct cpu_user_regs *regs)
 {
     struct serial_port *port = this_cpu(poll_port);
     struct ns16550 *uart = port->uart;
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index c6987973bf88..3a1fdaf7e35a 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -199,7 +199,7 @@  extern char *print_tainted(char *str);
 extern void add_taint(unsigned int taint);
 
 struct cpu_user_regs;
-void dump_execstate(struct cpu_user_regs *);
+void cf_check dump_execstate(struct cpu_user_regs *);
 
 void init_constructors(void);