[PATCH v3 2/3] xen: common: add ability to enable stack protector

[Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>]

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 three phases:

1. Pre-defined randomly-selected value.

2. Early use 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. Even when
get_cycles() is available, it's return value may be easily predicted,
especially on embedded systems, where boot time is quite consistent.

3. After hypervisor is sufficiently initialized, stack guard can be
set-up with get_random() function, which is expected to provide better
randomness.

Also this patch adds comment to asm-generic/random.h about stack
protector dependency on it.

Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk@xxxxxxxx>

---

Changes in v3:
 - Fixed coding style in stack-protector.h
 - Extended panic() message
 - Included missed random.h
 - Renamed Kconfig option
 - Used Andrew's suggestion for the Kconfig help text
 - Added "asmlinkage" attribute to __stack_chk_fail() to make Eclair
 happy
 - Initial stack guard value is random
 - Added LCG to generate stack guard value at early boot stages
 - Added comment to asm-generic/random.h about dependencies
 - Extended the commit message

Changes in v2:
 - Moved changes to EMBEDDED_EXTRA_CFLAGS into separate patch
 - Renamed stack_protector.c to stack-protector.c
 - Renamed stack_protector.h to stack-protector.h
 - Removed #ifdef CONFIG_X86 in stack-protector.h
 - Updated comment in stack-protector.h
   (also, we can't call boot_stack_chk_guard_setup() from asm code in
   general case, because it calls get_random() and get_random() may
   depend in per_cpu infrastructure, which is initialized later)
 - Fixed coding style
 - Moved CONFIG_STACK_PROTECTOR into newly added "Compiler options"
 submenu
 - Marked __stack_chk_guard as __ro_after_init
---
 xen/Makefile                      |  4 +++
 xen/common/Kconfig                | 15 ++++++++++
 xen/common/Makefile               |  1 +
 xen/common/stack-protector.c      | 47 +++++++++++++++++++++++++++++++
 xen/include/asm-generic/random.h  |  5 ++++
 xen/include/xen/stack-protector.h | 30 ++++++++++++++++++++
 6 files changed, 102 insertions(+)
 create mode 100644 xen/common/stack-protector.c
 create mode 100644 xen/include/xen/stack-protector.h

diff --git a/xen/Makefile b/xen/Makefile
index 34ed8c0fc7..0de0101fd0 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -432,7 +432,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 90268d9249..5676339a66 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -86,6 +86,9 @@ config HAS_UBSAN
 config HAS_VMAP
        bool
 
+config HAS_STACK_PROTECTOR
+       bool
+
 config MEM_ACCESS_ALWAYS_ON
        bool
 
@@ -213,6 +216,18 @@ config SPECULATIVE_HARDEN_LOCK
 
 endmenu
 
+menu "Compiler options"
+
+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 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 b279b09bfb..ceb5b2f32b 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -45,6 +45,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..922511555f
--- /dev/null
+++ b/xen/common/stack-protector.c
@@ -0,0 +1,47 @@
+// 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
+
+/* This function should be called from ASM only */
+void __init asmlinkage boot_stack_chk_guard_setup_early(void)
+{
+    /*
+     * 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;
+}
+
+void asmlinkage __stack_chk_fail(void)
+{
+    panic("Stack Protector integrity violation identified in %ps\n",
+         __builtin_return_address(0));
+}
diff --git a/xen/include/asm-generic/random.h b/xen/include/asm-generic/random.h
index d0d35dd217..7f6d8790c4 100644
--- a/xen/include/asm-generic/random.h
+++ b/xen/include/asm-generic/random.h
@@ -2,6 +2,11 @@
 #ifndef __ASM_GENERIC_RANDOM_H__
 #define __ASM_GENERIC_RANDOM_H__
 
+/*
+ * When implementing arch_get_random(), please make sure that
+ * it can provide random data before stack protector is initialized
+ * (i.e. before boot_stack_chk_guard_setup() is called).
+ */
 static inline unsigned int arch_get_random(void)
 {
     return 0;
diff --git a/xen/include/xen/stack-protector.h 
b/xen/include/xen/stack-protector.h
new file mode 100644
index 0000000000..bd324d9003
--- /dev/null
+++ b/xen/include/xen/stack-protector.h
@@ -0,0 +1,30 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef XEN__STACK_PROTECTOR_H
+#define XEN__STACK_PROTECTOR_H
+
+#ifdef CONFIG_STACK_PROTECTOR
+
+#include <xen/random.h>
+
+extern unsigned long __stack_chk_guard;
+
+/*
+ * This function should be always inlined. Also it should be called
+ * from a function that never returns or a function that has
+ * stack-protector disabled.
+ */
+static always_inline void boot_stack_chk_guard_setup(void)
+{
+    __stack_chk_guard = get_random();
+    if (BITS_PER_LONG == 64)
+        __stack_chk_guard |= ((unsigned long)get_random()) << 32;
+}
+
+#else
+
+static inline void boot_stack_chk_guard_setup(void) {}
+
+#endif /* CONFIG_STACK_PROTECTOR  */
+
+#endif /* XEN__STACK_PROTECTOR_H */
-- 
2.47.1