Both GCC and Clang support -fstack-protector feature, which add stack
canaries to functions where stack corruption is possible. This patch
makes general preparations to enable this feature on different
supported architectures:

 - Added CONFIG_HAS_STACK_PROTECTOR option so each architecture
   can enable this feature individually
 - Added user-selectable CONFIG_STACK_PROTECTOR option
 - Implemented code that sets up random stack canary and a basic
   handler for stack protector failures

Stack guard value is initialized in two phases:

1. Pre-defined randomly-selected value.

2. Own implementation of linear congruent random number generator. It
relies on get_cycles() being available very early. If get_cycles()
returns zero, it would leave pre-defined value from the previous step.

boot_stack_chk_guard_setup() is declared as always_inline to ensure
that it will not trigger stack protector by itself. And of course,
caller should ensure that stack protection code will not be reached
later. It is possible to call the same function from an ASM code by
introducing simple trampoline in stack-protector.c, but right now
there is no use case for such trampoline.

As __stack_chk_fail() is not called by Xen source code directly, and
only called by compiler-generated code, it does not needed to be
declared separately. So we need separate MISRA deviation for it.

Signed-off-by: Volodymyr Babchuk <volodymyr_babc...@epam.com>

---

Changes in v8:
 - Code formatting fixes
 - Added an explicit MISRA deviation for __stack_chk_fail()
 - Marked __stack_chk_fail() as noreturn

Changes in v7:
 - declared boot_stack_chk_guard_setup as always_inline
 - moved `#ifdef CONFIG_STACK_PROTECTOR` inside the function

Changes in v6:
 - boot_stack_chk_guard_setup() moved to stack-protector.h
 - Removed Andrew's r-b tag

Changes in v5:
 - Fixed indentation
 - Added stack-protector.h
---
 docs/misra/safe.json              |  8 +++++++
 xen/Makefile                      |  4 ++++
 xen/common/Kconfig                | 15 ++++++++++++
 xen/common/Makefile               |  1 +
 xen/common/stack-protector.c      | 22 +++++++++++++++++
 xen/include/xen/stack-protector.h | 39 +++++++++++++++++++++++++++++++
 6 files changed, 89 insertions(+)
 create mode 100644 xen/common/stack-protector.c
 create mode 100644 xen/include/xen/stack-protector.h

diff --git a/docs/misra/safe.json b/docs/misra/safe.json
index 3d68b59169..e249bcbf81 100644
--- a/docs/misra/safe.json
+++ b/docs/misra/safe.json
@@ -108,6 +108,14 @@
         },
         {
             "id": "SAF-13-safe",
+            "analyser": {
+                "eclair": "MC3A2.R8.4"
+            },
+            "name": "Rule 8.4: compiler-called function",
+            "text": "A function, for which compiler generates calls to do not 
need to have a visible declaration prior to its definition."
+        },
+        {
+            "id": "SAF-14-safe",
             "analyser": {},
             "name": "Sentinel",
             "text": "Next ID to be used"
diff --git a/xen/Makefile b/xen/Makefile
index 58fafab33d..8fc4e042ff 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -435,7 +435,11 @@ else
 CFLAGS_UBSAN :=
 endif
 
+ifeq ($(CONFIG_STACK_PROTECTOR),y)
+CFLAGS += -fstack-protector
+else
 CFLAGS += -fno-stack-protector
+endif
 
 ifeq ($(CONFIG_LTO),y)
 CFLAGS += -flto
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 06ae9751aa..42a2b6a03f 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -100,6 +100,9 @@ config HAS_PMAP
 config HAS_SCHED_GRANULARITY
        bool
 
+config HAS_STACK_PROTECTOR
+       bool
+
 config HAS_UBSAN
        bool
 
@@ -233,6 +236,18 @@ config SPECULATIVE_HARDEN_LOCK
 
 endmenu
 
+menu "Other hardening"
+
+config STACK_PROTECTOR
+       bool "Stack protector"
+       depends on HAS_STACK_PROTECTOR
+       help
+         Enable the Stack Protector compiler hardening option. This inserts a
+         canary value in the stack frame of functions, and performs an 
integrity
+         check on function exit.
+
+endmenu
+
 config DIT_DEFAULT
        bool "Data Independent Timing default"
        depends on HAS_DIT
diff --git a/xen/common/Makefile b/xen/common/Makefile
index 9da8a7244d..f625031d16 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -46,6 +46,7 @@ obj-y += shutdown.o
 obj-y += softirq.o
 obj-y += smp.o
 obj-y += spinlock.o
+obj-$(CONFIG_STACK_PROTECTOR) += stack-protector.o
 obj-y += stop_machine.o
 obj-y += symbols.o
 obj-y += tasklet.o
diff --git a/xen/common/stack-protector.c b/xen/common/stack-protector.c
new file mode 100644
index 0000000000..2115912c3b
--- /dev/null
+++ b/xen/common/stack-protector.c
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#include <xen/init.h>
+#include <xen/lib.h>
+#include <xen/random.h>
+#include <xen/time.h>
+
+/*
+ * Initial value is chosen by a fair dice roll.
+ * It will be updated during boot process.
+ */
+#if BITS_PER_LONG == 32
+unsigned long __ro_after_init __stack_chk_guard = 0xdd2cc927UL;
+#else
+unsigned long __ro_after_init __stack_chk_guard = 0x2d853605a4d9a09cUL;
+#endif
+
+/* SAF-13-safe compiler-called function */
+void noreturn __stack_chk_fail(void)
+{
+    dump_execution_state();
+    panic("Stack Protector integrity violation identified\n");
+}
diff --git a/xen/include/xen/stack-protector.h 
b/xen/include/xen/stack-protector.h
new file mode 100644
index 0000000000..931affd919
--- /dev/null
+++ b/xen/include/xen/stack-protector.h
@@ -0,0 +1,39 @@
+#ifndef __XEN_STACK_PROTECTOR_H__
+#define __XEN_STACK_PROTECTOR_H__
+
+extern unsigned long __stack_chk_guard;
+
+/*
+ * This function should be called from a C function that escapes stack
+ * canary tracking (by calling reset_stack_and_jump() for example).
+ */
+static always_inline void boot_stack_chk_guard_setup(void)
+{
+#ifdef CONFIG_STACK_PROTECTOR
+
+    /*
+     * Linear congruent generator (X_n+1 = X_n * a + c).
+     *
+     * Constant is taken from "Tables Of Linear Congruential
+     * Generators Of Different Sizes And Good Lattice Structure" by
+     * Pierre L’Ecuyer.
+     */
+#if BITS_PER_LONG == 32
+    const unsigned long a = 2891336453UL;
+#else
+    const unsigned long a = 2862933555777941757UL;
+#endif
+    const unsigned long c = 1;
+
+    unsigned long cycles = get_cycles();
+
+    /* Use the initial value if we can't generate random one */
+    if ( !cycles )
+        return;
+
+    __stack_chk_guard = cycles * a + c;
+
+#endif /* CONFIG_STACK_PROTECTOR */
+}
+
+#endif /* __XEN_STACK_PROTECTOR_H__ */
-- 
2.48.1

Reply via email to