Basic overview
---------------
This is a RFC patch series for enabling kernel control flow integrity on
riscv architecture. This patch series enables kernel control flow
integrity using proposed riscv cpu extensions `zicfilp` and `zicfiss` [1].
`zicfilp` enforces that all indirect calls and jumps must land on a landing
pad instruction (`lpad`). Additionally `lpad` has 20bit encoded value as
part of instruction and cpu will check this 20bit value with t2/x7 register
, if they mismatch then cpu will raise an exception `software check
exception` (a new exception with cause=18). In this patch series, a
constant label value of 0x1 is used. As series will mature, it will switch
to a 20 bit truncated hash over function signature. Label based on function
signature allows stricter control flow and fewer call/jmp locations from a
callsite.
`zicfiss` protects the return path from functions where return relies on
obtaining return address from stack which is corruptible. `zicfiss`
provides a shadow stack which can be used by software to place return
addresses on shadow stack and while returning from function it can be used
to compare against return address from regular stack. If they dont match,
cpu will raise software check exception. `zicfiss` based shadow stack are
protected against tampering using special page table encodings (please
refer to [1])
To obtain more details about `zicfiss` and `zicfilp` ISA extension, please
refer to [1]. There is an ongoing patchsets for enabling this feature for
user mode software here [2]
Enabling on kernel
===================
This patch series introduces new riscv config `CONFIG_RISCV_KERNEL_CFI`.
If this config is selected, it turns on
- forward control flow for kernel using `zicfilp`
- selects `CONFIG_SHADOW_CALL_STACK` /w `CONFIG_DYNAMIC_SCS` to enable
backward control flow.
forward control flow for kernel
================================
This patch series simply compiles kernel with `march=_zicfilp` compiler
option. Currently toolchain uses constant label scheme of label = 0x1.
This patch series manually fixes some of the assembly callsites and
sequences to make sure they are not breaking rules setup by `zicfilp`.
backward control flow for kernel
=================================
There is an existing support for riscv kernel for shadow call stack [3],
which is a software based shadow stack and uses clang /w instrumentation
to push/pop return address in prolog and epilog of functions. Although
software based shadow stack lacks memory protections and thus suffers from
same issue of return address susceptible to hijacking. shadow call stack
uses `CONFIG_SHADOW_CALL_STACK` /w option of `CONFIG_DYNAMIC_SCS` so that
hardware vendors hook into the flow to provide stronger guarantees. This
patch uses `CONFIG_SHADOW_CALL_STACK` flow along with `CONFIG_DYNAMIC_SCS`
to enable return control flow integrity on riscv kernel.
[1] - https://github.com/riscv/riscv-cfi
[2] - https://lore.kernel.org/all/[email protected]/
[3] - https://lore.kernel.org/all/[email protected]/
All call sites written in asm which will be converted to indirect call
form, they need to setup label register (t2/x7) with correct label.
Currently kernel is enabled with consant label of 0x1 for all functions.
Thus label is setup with 0x1 at call site. Eventually when hash over
function signature based label is adopted, such callsites in asm needs
to b updated as well. We need better scheme for that (some macro)
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/kernel/entry.S | 2 ++
arch/riscv/kernel/head.S | 1 +
arch/riscv/lib/clear_page.S | 1 +
3 files changed, 4 insertions(+)
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index be07355b9eff..a35050a3e0ea 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -219,6 +219,7 @@ SYM_CODE_START_LOCAL(handle_kernel_stack_overflow)
REG_S s4, PT_CAUSE(sp)
REG_S s5, PT_TP(sp)
move a0, sp
+ lui t2,0x1
tail handle_bad_stack
SYM_CODE_END(handle_kernel_stack_overflow)
ASM_NOKPROBE(handle_kernel_stack_overflow)
@@ -258,6 +259,7 @@ SYM_FUNC_START(call_on_irq_stack)
load_per_cpu t0, irq_stack_ptr, t1
li t1, IRQ_STACK_SIZE
add sp, t0, t1
+ lui t2, 0x1
jalr a1
/* Switch back to the thread shadow call stack */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 4236a69c35cb..6c311517c3b5 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -165,6 +165,7 @@ secondary_start_sbi:
#endif
call .Lsetup_trap_vector
scs_load_current
+ lui t2, 0x1
tail smp_callin
#endif /* CONFIG_SMP */
diff --git a/arch/riscv/lib/clear_page.S b/arch/riscv/lib/clear_page.S
index 20ff03f5b0f2..16e63ea91baa 100644
--- a/arch/riscv/lib/clear_page.S
+++ b/arch/riscv/lib/clear_page.S
@@ -69,6 +69,7 @@ SYM_FUNC_START(clear_page)
ret
.Lno_zicboz:
li a1, 0
+ lui t2, 0x1
tail __memset
SYM_FUNC_END(clear_page)
EXPORT_SYMBOL(clear_page)
--
2.43.2
If compiled for riscv and dynamic scs is turned on, place shadow stack in
`.shadowstack` section. Arch specific linker script can place logic to
protect this section against regular stores.
Signed-off-by: Deepak Gupta <[email protected]>
---
include/linux/init_task.h | 5 +++++
init/init_task.c | 12 ++++++++++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/include/linux/init_task.h b/include/linux/init_task.h
index bccb3f1f6262..f025022e1164 100644
--- a/include/linux/init_task.h
+++ b/include/linux/init_task.h
@@ -40,4 +40,9 @@ extern struct cred init_cred;
/* Attach to the thread_info data structure for proper alignment */
#define __init_thread_info __section(".data..init_thread_info")
+#if defined(CONFIG_RISCV) && defined(CONFIG_DYNAMIC_SCS)
+/* init shadow stack page */
+#define __init_shadow_stack __section(".shadowstack..init")
+#endif
+
#endif
diff --git a/init/init_task.c b/init/init_task.c
index 4daee6d761c8..3c0a01455978 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -52,10 +52,18 @@ static struct sighand_struct init_sighand = {
};
#ifdef CONFIG_SHADOW_CALL_STACK
-unsigned long init_shadow_call_stack[SCS_SIZE / sizeof(long)] = {
+unsigned long init_shadow_call_stack[SCS_SIZE / sizeof(long)]
+#if defined(CONFIG_RISCV) && defined(CONFIG_DYNAMIC_SCS)
+ /* RISC-V dynamic SCS implements shadow stack and must go in special section */
+ __init_shadow_stack = {
+ [0] = SCS_END_MAGIC
+};
+#else
+ = {
[(SCS_SIZE / sizeof(long)) - 1] = SCS_END_MAGIC
};
-#endif
+#endif /* CONFIG_RISCV && CONFIG_DYNAMIC_SCS */
+#endif /* CONFIG_SHADOW_CALL_STACK */
/*
* Set up the first task table, touch at your own risk!. Base=0,
--
2.43.2
Under CONFIG_SHADOW_CALL_STACK, shadow call stack goes into data section.
Although with CONFIG_DYNAMIC_SCS on riscv, hardware assisted shadow stack
are used. Hardware assisted shadow stack on riscv uses PTE.R=0, PTE.W=1 &
PTE.X=0 encodings. Without CONFIG_DYNAMIC_SCS, shadow stack for init is
placed in data section and thus regular read/write encodings are applied
to it. Although with with CONFIG_DYNAMIC_SCS, they need to go into
different section. This change places it into `.shadowstack` section.
As part of this change early boot code (`setup_vm`), applies appropriate
PTE encodings to shadow call stack for init placed in `.shadowstack`
section.
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/include/asm/pgtable.h | 4 ++++
arch/riscv/include/asm/sections.h | 22 +++++++++++++++++++++
arch/riscv/include/asm/thread_info.h | 10 ++++++++--
arch/riscv/kernel/vmlinux.lds.S | 12 ++++++++++++
arch/riscv/mm/init.c | 29 +++++++++++++++++++++-------
5 files changed, 68 insertions(+), 9 deletions(-)
diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 9f8ea0e33eb1..3409b250390d 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -197,6 +197,10 @@ extern struct pt_alloc_ops pt_ops __initdata;
#define PAGE_KERNEL_READ_EXEC __pgprot((_PAGE_KERNEL & ~_PAGE_WRITE) \
| _PAGE_EXEC)
+#ifdef CONFIG_DYNAMIC_SCS
+#define PAGE_KERNEL_SHADOWSTACK __pgprot(_PAGE_KERNEL & ~(_PAGE_READ | _PAGE_EXEC))
+#endif
+
#define PAGE_TABLE __pgprot(_PAGE_TABLE)
#define _PAGE_IOREMAP ((_PAGE_KERNEL & ~_PAGE_MTMASK) | _PAGE_IO)
diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
index a393d5035c54..4c4154d0021e 100644
--- a/arch/riscv/include/asm/sections.h
+++ b/arch/riscv/include/asm/sections.h
@@ -14,6 +14,10 @@ extern char __init_data_begin[], __init_data_end[];
extern char __init_text_begin[], __init_text_end[];
extern char __alt_start[], __alt_end[];
extern char __exittext_begin[], __exittext_end[];
+#ifdef CONFIG_DYNAMIC_SCS
+extern char __init_shstk_start[], __init_shstk_end[];
+#endif
+extern char __end_srodata[];
static inline bool is_va_kernel_text(uintptr_t va)
{
@@ -31,4 +35,22 @@ static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
return va >= start && va < end;
}
+#ifdef CONFIG_DYNAMIC_SCS
+static inline bool is_va_init_shadow_stack_early(uintptr_t va)
+{
+ uintptr_t start = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_start));
+ uintptr_t end = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_end));
+
+ return va >= start && va < end;
+}
+
+static inline bool is_va_init_shadow_stack(uintptr_t va)
+{
+ uintptr_t start = (uintptr_t)(__init_shstk_start);
+ uintptr_t end = (uintptr_t)(__init_shstk_end);
+
+ return va >= start && va < end;
+}
+#endif
+
#endif /* __ASM_SECTIONS_H */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 5d473343634b..7ae28d627f84 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -63,12 +63,18 @@ struct thread_info {
};
#ifdef CONFIG_SHADOW_CALL_STACK
+#ifdef CONFIG_DYNAMIC_SCS
#define INIT_SCS \
- .scs_base = init_shadow_call_stack, \
+ .scs_base = init_shadow_call_stack, \
+ .scs_sp = &init_shadow_call_stack[SCS_SIZE / sizeof(long)],
+#else
+#define INIT_SCS \
+ .scs_base = init_shadow_call_stack, \
.scs_sp = init_shadow_call_stack,
+#endif /* CONFIG_DYNAMIC_SCS */
#else
#define INIT_SCS
-#endif
+#endif /* CONFIG_SHADOW_CALL_STACK */
/*
* macros/functions for gaining access to the thread information structure
diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
index 002ca58dd998..cccc51f845ab 100644
--- a/arch/riscv/kernel/vmlinux.lds.S
+++ b/arch/riscv/kernel/vmlinux.lds.S
@@ -126,6 +126,18 @@ SECTIONS
*(.srodata*)
}
+ . = ALIGN(SECTION_ALIGN);
+ __end_srodata = .;
+
+#ifdef CONFIG_DYNAMIC_SCS
+ .shadowstack : AT(ADDR(.shadowstack) - LOAD_OFFSET){
+ __init_shstk_start = .;
+ KEEP(*(.shadowstack..init))
+ . = __init_shstk_start + PAGE_SIZE;
+ __init_shstk_end = .;
+ }
+#endif
+
. = ALIGN(SECTION_ALIGN);
_data = .;
diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
index fe8e159394d8..5b6f0cfa5719 100644
--- a/arch/riscv/mm/init.c
+++ b/arch/riscv/mm/init.c
@@ -713,14 +713,22 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
return PAGE_KERNEL_READ;
+#ifdef CONFIG_DYNAMIC_SCS
+ /* If init task's shadow stack va, return write only page protections */
+ if (IS_ENABLED(CONFIG_64BIT) && is_va_init_shadow_stack(va)) {
+ pr_info("Shadow stack protections are being applied to for init\n");
+ return PAGE_KERNEL_SHADOWSTACK;
+ }
+#endif
+
return PAGE_KERNEL;
}
void mark_rodata_ro(void)
{
- set_kernel_memory(__start_rodata, _data, set_memory_ro);
+ set_kernel_memory(__start_rodata, __end_srodata, set_memory_ro);
if (IS_ENABLED(CONFIG_64BIT))
- set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
+ set_kernel_memory(lm_alias(__start_rodata), lm_alias(__end_srodata),
set_memory_ro);
}
#else
@@ -913,14 +921,21 @@ static void __init create_kernel_page_table(pgd_t *pgdir,
static void __init create_kernel_page_table(pgd_t *pgdir, bool early)
{
uintptr_t va, end_va;
+ pgprot_t prot;
end_va = kernel_map.virt_addr + kernel_map.size;
- for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE)
+ for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE) {
+ prot = PAGE_KERNEL_EXEC;
+#ifdef CONFIG_DYNAMIC_SCS
+ if (early && is_va_init_shadow_stack_early(va))
+ prot = PAGE_KERNEL_SHADOWSTACK;
+#endif
create_pgd_mapping(pgdir, va,
- kernel_map.phys_addr + (va - kernel_map.virt_addr),
- PMD_SIZE,
- early ?
- PAGE_KERNEL_EXEC : pgprot_from_va(va));
+ kernel_map.phys_addr + (va - kernel_map.virt_addr),
+ PMD_SIZE,
+ early ?
+ prot : pgprot_from_va(va));
+ }
}
#endif
--
2.43.2
If shadow stack have memory protections from underlying cpu, use those
protections. RISCV uses PAGE_KERNEL_SHADOWSTACK to vmalloc such shadow
stack pages. Shadow stack pages on RISCV grows downwards like regular
stack. Clang based software shadow call stack grows low to high address.
Thus this patch addresses some of those needs due to opposite direction
of shadow stack. Furthermore, RISCV hw shadow stack can't be memset
because memset uses normal stores. Lastly to store magic word at base of
shadow stack, arch specific shadow stack store has to be performed.
Signed-off-by: Deepak Gupta <[email protected]>
---
include/linux/scs.h | 48 +++++++++++++++++++++++++++++++++------------
kernel/scs.c | 28 ++++++++++++++++++++++----
2 files changed, 59 insertions(+), 17 deletions(-)
diff --git a/include/linux/scs.h b/include/linux/scs.h
index 4ab5bdc898cf..3a31433532d1 100644
--- a/include/linux/scs.h
+++ b/include/linux/scs.h
@@ -12,6 +12,7 @@
#include <linux/poison.h>
#include <linux/sched.h>
#include <linux/sizes.h>
+#include <asm/scs.h>
#ifdef CONFIG_SHADOW_CALL_STACK
@@ -31,6 +32,29 @@ void scs_init(void);
int scs_prepare(struct task_struct *tsk, int node);
void scs_release(struct task_struct *tsk);
+#ifdef CONFIG_DYNAMIC_SCS
+/* dynamic_scs_enabled set to true if RISCV dynamic SCS */
+#ifdef CONFIG_RISCV
+DECLARE_STATIC_KEY_TRUE(dynamic_scs_enabled);
+#else
+DECLARE_STATIC_KEY_FALSE(dynamic_scs_enabled);
+#endif
+#endif
+
+static inline bool scs_is_dynamic(void)
+{
+ if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
+ return false;
+ return static_branch_likely(&dynamic_scs_enabled);
+}
+
+static inline bool scs_is_enabled(void)
+{
+ if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
+ return true;
+ return scs_is_dynamic();
+}
+
static inline void scs_task_reset(struct task_struct *tsk)
{
/*
@@ -42,6 +66,9 @@ static inline void scs_task_reset(struct task_struct *tsk)
static inline unsigned long *__scs_magic(void *s)
{
+ if (scs_is_dynamic())
+ return (unsigned long *)(s);
+
return (unsigned long *)(s + SCS_SIZE) - 1;
}
@@ -50,23 +77,18 @@ static inline bool task_scs_end_corrupted(struct task_struct *tsk)
unsigned long *magic = __scs_magic(task_scs(tsk));
unsigned long sz = task_scs_sp(tsk) - task_scs(tsk);
- return sz >= SCS_SIZE - 1 || READ_ONCE_NOCHECK(*magic) != SCS_END_MAGIC;
-}
-
-DECLARE_STATIC_KEY_FALSE(dynamic_scs_enabled);
+ if (scs_is_dynamic())
+ sz = (task_scs(tsk) + SCS_SIZE) - task_scs_sp(tsk);
-static inline bool scs_is_dynamic(void)
-{
- if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
- return false;
- return static_branch_likely(&dynamic_scs_enabled);
+ return sz >= SCS_SIZE - 1 || READ_ONCE_NOCHECK(*magic) != SCS_END_MAGIC;
}
-static inline bool scs_is_enabled(void)
+static inline void __scs_store_magic(unsigned long *s, unsigned long magic_val)
{
- if (!IS_ENABLED(CONFIG_DYNAMIC_SCS))
- return true;
- return scs_is_dynamic();
+ if (scs_is_dynamic())
+ arch_scs_store(s, magic_val);
+ else
+ *__scs_magic(s) = SCS_END_MAGIC;
}
#else /* CONFIG_SHADOW_CALL_STACK */
diff --git a/kernel/scs.c b/kernel/scs.c
index d7809affe740..e447483fa9f4 100644
--- a/kernel/scs.c
+++ b/kernel/scs.c
@@ -13,8 +13,13 @@
#include <linux/vmstat.h>
#ifdef CONFIG_DYNAMIC_SCS
+/* dynamic_scs_enabled set to true if RISCV dynamic SCS */
+#ifdef CONFIG_RISCV
+DEFINE_STATIC_KEY_TRUE(dynamic_scs_enabled);
+#else
DEFINE_STATIC_KEY_FALSE(dynamic_scs_enabled);
#endif
+#endif
static void __scs_account(void *s, int account)
{
@@ -32,19 +37,29 @@ static void *__scs_alloc(int node)
{
int i;
void *s;
+ pgprot_t prot = PAGE_KERNEL;
+
+ if (scs_is_dynamic())
+ prot = PAGE_KERNEL_SHADOWSTACK;
for (i = 0; i < NR_CACHED_SCS; i++) {
s = this_cpu_xchg(scs_cache[i], NULL);
if (s) {
s = kasan_unpoison_vmalloc(s, SCS_SIZE,
KASAN_VMALLOC_PROT_NORMAL);
- memset(s, 0, SCS_SIZE);
+/*
+ * If either of them undefined, its safe to memset. Else memset is not
+ * possible. memset constitutes stores and stores to shadow stack memory
+ * are disallowed and will fault.
+ */
+ if (!scs_is_dynamic())
+ memset(s, 0, SCS_SIZE);
goto out;
}
}
s = __vmalloc_node_range(SCS_SIZE, 1, VMALLOC_START, VMALLOC_END,
- GFP_SCS, PAGE_KERNEL, 0, node,
+ GFP_SCS, prot, 0, node,
__builtin_return_address(0));
out:
@@ -59,7 +74,7 @@ void *scs_alloc(int node)
if (!s)
return NULL;
- *__scs_magic(s) = SCS_END_MAGIC;
+ __scs_store_magic(__scs_magic(s), SCS_END_MAGIC);
/*
* Poison the allocation to catch unintentional accesses to
@@ -122,7 +137,12 @@ int scs_prepare(struct task_struct *tsk, int node)
if (!s)
return -ENOMEM;
- task_scs(tsk) = task_scs_sp(tsk) = s;
+ task_scs(tsk) = s;
+ if (scs_is_dynamic())
+ task_scs_sp(tsk) = s + SCS_SIZE;
+ else
+ task_scs_sp(tsk) = s;
+
return 0;
}
--
2.43.2
Adding support for dynamic shadow call stack on riscv. zicfiss ISA extn.
enables protection for shadow stack against stray writes. This patch
enables scs_* macros to use zicfiss shadow stack pointer (CSR_SSP) instead
of relying on `gp`.
Since zicfiss based shadow stack needs to have correct encoding set in PTE
init shadow stack can't be established too early. It has to be setup after
`setup_vm` is called. Thus `scs_load_init_stack` is noped out if
CONFIG_DYNAMIC_SCS is selected.
Adds `arch_scs_store` that can be used in generic scs magic store routine.
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/include/asm/asm.h | 2 +-
arch/riscv/include/asm/scs.h | 47 +++++++++++++++++++++++++++++-------
arch/riscv/kernel/entry.S | 14 +++++------
arch/riscv/kernel/head.S | 4 +--
4 files changed, 48 insertions(+), 19 deletions(-)
diff --git a/arch/riscv/include/asm/asm.h b/arch/riscv/include/asm/asm.h
index 776354895b81..0304978ea4e4 100644
--- a/arch/riscv/include/asm/asm.h
+++ b/arch/riscv/include/asm/asm.h
@@ -109,7 +109,7 @@
REG_L \dst, 0(\dst)
.endm
-#ifdef CONFIG_SHADOW_CALL_STACK
+#if defined(CONFIG_SHADOW_CALL_STACK) && !defined(CONFIG_DYNAMIC_SCS)
/* gp is used as the shadow call stack pointer instead */
.macro load_global_pointer
.endm
diff --git a/arch/riscv/include/asm/scs.h b/arch/riscv/include/asm/scs.h
index 0e45db78b24b..14ef539922c2 100644
--- a/arch/riscv/include/asm/scs.h
+++ b/arch/riscv/include/asm/scs.h
@@ -9,46 +9,75 @@
/* Load init_shadow_call_stack to gp. */
.macro scs_load_init_stack
+#ifndef CONFIG_DYNAMIC_SCS
la gp, init_shadow_call_stack
XIP_FIXUP_OFFSET gp
+#endif
.endm
/* Load the per-CPU IRQ shadow call stack to gp. */
-.macro scs_load_irq_stack tmp
+.macro scs_load_irq_stack tmp tmp1
+#ifdef CONFIG_DYNAMIC_SCS
+ load_per_cpu \tmp1, irq_shadow_call_stack_ptr, \tmp
+ li \tmp, 4096
+ add \tmp, \tmp, \tmp1
+ csrw CSR_SSP, \tmp
+#else
load_per_cpu gp, irq_shadow_call_stack_ptr, \tmp
+#endif
.endm
/* Load task_scs_sp(current) to gp. */
-.macro scs_load_current
+.macro scs_load_current tmp
+#ifdef CONFIG_DYNAMIC_SCS
+ REG_L \tmp, TASK_TI_SCS_SP(tp)
+ csrw CSR_SSP, \tmp
+#else
REG_L gp, TASK_TI_SCS_SP(tp)
+#endif
.endm
/* Load task_scs_sp(current) to gp, but only if tp has changed. */
-.macro scs_load_current_if_task_changed prev
+.macro scs_load_current_if_task_changed prev tmp
beq \prev, tp, _skip_scs
- scs_load_current
+ scs_load_current \tmp
_skip_scs:
.endm
/* Save gp to task_scs_sp(current). */
-.macro scs_save_current
+.macro scs_save_current tmp
+#ifdef CONFIG_DYNAMIC_SCS
+ csrr \tmp, CSR_SSP
+ REG_S \tmp, TASK_TI_SCS_SP(tp)
+#else
REG_S gp, TASK_TI_SCS_SP(tp)
+#endif
.endm
#else /* CONFIG_SHADOW_CALL_STACK */
.macro scs_load_init_stack
.endm
-.macro scs_load_irq_stack tmp
+.macro scs_load_irq_stack tmp tmp1
.endm
-.macro scs_load_current
+.macro scs_load_current tmp
.endm
-.macro scs_load_current_if_task_changed prev
+.macro scs_load_current_if_task_changed prev tmp
.endm
-.macro scs_save_current
+.macro scs_save_current tmp
.endm
#endif /* CONFIG_SHADOW_CALL_STACK */
#endif /* __ASSEMBLY__ */
+#ifdef CONFIG_DYNAMIC_SCS
+#define arch_scs_store(ss_addr, magic_val) \
+ asm volatile ("ssamoswap.d %0, %2, %1" \
+ : "=r" (magic_val), "+A" (*ss_addr) \
+ : "r" (magic_val) \
+ : "memory")
+#else
+#define arch_scs_store(ss_addr, magic_val)
+#endif
+
#endif /* _ASM_SCS_H */
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index a35050a3e0ea..0262b46ab064 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -81,7 +81,7 @@ SYM_CODE_START(handle_exception)
load_global_pointer
/* Load the kernel shadow call stack pointer if coming from userspace */
- scs_load_current_if_task_changed s5
+ scs_load_current_if_task_changed s5 t0
#ifdef CONFIG_RISCV_ISA_V_PREEMPTIVE
move a0, sp
@@ -135,7 +135,7 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
REG_S s0, TASK_TI_KERNEL_SP(tp)
/* Save the kernel shadow call stack pointer */
- scs_save_current
+ scs_save_current t0
/*
* Save TP into the scratch register , so we can find the kernel data
@@ -252,8 +252,8 @@ SYM_FUNC_START(call_on_irq_stack)
addi s0, sp, STACKFRAME_SIZE_ON_STACK
/* Switch to the per-CPU shadow call stack */
- scs_save_current
- scs_load_irq_stack t0
+ scs_save_current t0
+ scs_load_irq_stack t0 t1
/* Switch to the per-CPU IRQ stack and call the handler */
load_per_cpu t0, irq_stack_ptr, t1
@@ -263,7 +263,7 @@ SYM_FUNC_START(call_on_irq_stack)
jalr a1
/* Switch back to the thread shadow call stack */
- scs_load_current
+ scs_load_current t0
/* Switch back to the thread stack and restore ra and s0 */
addi sp, s0, -STACKFRAME_SIZE_ON_STACK
@@ -305,7 +305,7 @@ SYM_FUNC_START(__switch_to)
REG_S s10, TASK_THREAD_S10_RA(a3)
REG_S s11, TASK_THREAD_S11_RA(a3)
/* Save the kernel shadow call stack pointer */
- scs_save_current
+ scs_save_current t0
/* Restore context from next->thread */
REG_L ra, TASK_THREAD_RA_RA(a4)
REG_L sp, TASK_THREAD_SP_RA(a4)
@@ -324,7 +324,7 @@ SYM_FUNC_START(__switch_to)
/* The offset of thread_info in task_struct is zero. */
move tp, a1
/* Switch to the next shadow call stack */
- scs_load_current
+ scs_load_current t0
ret
SYM_FUNC_END(__switch_to)
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index 6c311517c3b5..bc248c137c90 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -164,7 +164,7 @@ secondary_start_sbi:
call relocate_enable_mmu
#endif
call .Lsetup_trap_vector
- scs_load_current
+ scs_load_current t0
lui t2, 0x1
tail smp_callin
#endif /* CONFIG_SMP */
@@ -313,7 +313,7 @@ SYM_CODE_START(_start_kernel)
la tp, init_task
la sp, init_thread_union + THREAD_SIZE
addi sp, sp, -PT_SIZE_ON_STACK
- scs_load_current
+ scs_load_current t0
#ifdef CONFIG_KASAN
call kasan_early_init
--
2.43.2
Handwritten `__memset` asm routine performs certain static jumps within
function and uses `a5` to do that. This would require a landing pad
instruction at the target. Since its static jump and no memory load is
involved, use `t2` instead which is exempt from requiring a landing pad.
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/lib/memset.S | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/riscv/lib/memset.S b/arch/riscv/lib/memset.S
index 35f358e70bdb..e129ebf66986 100644
--- a/arch/riscv/lib/memset.S
+++ b/arch/riscv/lib/memset.S
@@ -56,12 +56,12 @@ SYM_FUNC_START(__memset)
/* Jump into loop body */
/* Assumes 32-bit instruction lengths */
- la a5, 3f
+ la t2, 3f
#ifdef CONFIG_64BIT
srli a4, a4, 1
#endif
- add a5, a5, a4
- jr a5
+ add t2, t2, a4
+ jr t2
3:
REG_S a1, 0(t0)
REG_S a1, SZREG(t0)
--
2.43.2
Defines `CONFIG_RISCV_KERNEL_CFI` and selects SHADOW_CALL_STACK
and DYNAMIC_SCS both so that zicfiss can be wired up.
Makefile checks if CONFIG_RISCV_KERNEL_CFI is enabled, then light
up zicfiss and zicfilp compiler flags.
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
arch/riscv/Makefile | 6 ++++++
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index be09c8836d56..5276598bb773 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -193,7 +193,7 @@ config GCC_SUPPORTS_DYNAMIC_FTRACE
depends on $(cc-option,-fpatchable-function-entry=8)
config HAVE_SHADOW_CALL_STACK
- def_bool $(cc-option,-fsanitize=shadow-call-stack)
+ def_bool $(cc-option,-fsanitize=shadow-call-stack) || $(cc-option,-mabi=lp64 -march=rv64ima_zicfilp_zicfiss)
# https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a484e843e6eeb51f0cb7b8819e50da6d2444d769
depends on $(ld-option,--no-relax-gp)
@@ -211,6 +211,30 @@ config ARCH_HAS_BROKEN_DWARF5
# https://github.com/llvm/llvm-project/commit/7ffabb61a5569444b5ac9322e22e5471cc5e4a77
depends on LD_IS_LLD && LLD_VERSION < 180000
+config RISCV_KERNEL_CFI
+ def_bool n
+ bool "hw assisted riscv kernel control flow integrity (kcfi)"
+ depends on 64BIT && $(cc-option,-mabi=lp64 -march=rv64ima_zicfilp_zicfiss)
+ select ARCH_SUPPORTS_SHADOW_CALL_STACK
+ select SHADOW_CALL_STACK
+ select DYNAMIC_SCS
+ help
+ Provides CPU assisted control flow integrity to for riscv kernel.
+ Control flow integrity is provided by implementing shadow stack for
+ backward edge and indirect branch tracking for forward edge. Shadow
+ stack protection is a hardware feature that detects function return
+ address corruption. This helps mitigate ROP attacks. RISCV_KERNEL_CFI
+ selects CONFIG_SHADOW_CALL_STACK which uses software based shadow
+ stack but is unprotected against stray writes. Selecting RISCV_KERNEL_CFI
+ will select CONFIG_DYNAMIC_SCS and will enable hardware assisted shadow
+ stack protection against stray writes.
+ Indirect branch tracking enforces that all indirect branches must land
+ on a landing pad instruction else CPU will fault. This enables forward
+ control flow (call/jmp) protection in kernel and restricts all indirect
+ call or jump in kernel to a landing pad instruction which mostly likely
+ will be start of the function.
+ default n
+
config ARCH_MMAP_RND_BITS_MIN
default 18 if 64BIT
default 8
@@ -639,6 +663,16 @@ config RISCV_ISA_ZICBOZ
If you don't know what to do here, say Y.
+config TOOLCHAIN_HAS_ZICFILP
+ bool
+ default y
+ depends on 64BIT && $(cc-option,-mabi=lp64 -march=rv64ima_zicfilp)
+
+config TOOLCHAIN_HAS_ZICFISS
+ bool
+ default y
+ depends on 64BIT && $(cc-option,-mabi=lp64 -march=rv64ima_zicfiss)
+
config TOOLCHAIN_HAS_ZIHINTPAUSE
bool
default y
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 5b3115a19852..ae156e37e886 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -58,8 +58,10 @@ else ifeq ($(CONFIG_LTO_CLANG),y)
endif
ifeq ($(CONFIG_SHADOW_CALL_STACK),y)
+ifndef CONFIG_DYNAMIC_SCS
KBUILD_LDFLAGS += --no-relax-gp
endif
+endif
# ISA string setting
riscv-march-$(CONFIG_ARCH_RV32I) := rv32ima
@@ -78,6 +80,10 @@ endif
# Check if the toolchain supports Zihintpause extension
riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE) := $(riscv-march-y)_zihintpause
+ifeq ($(CONFIG_RISCV_KERNEL_CFI),y)
+riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICFILP) := $(riscv-march-y)_zicfilp
+riscv-march-$(CONFIG_TOOLCHAIN_HAS_ZICFISS) := $(riscv-march-y)_zicfiss
+endif
# Remove F,D,V from isa string for all. Keep extensions between "fd" and "v" by
# matching non-v and non-multi-letter extensions out with the filter ([^v_]*)
KBUILD_CFLAGS += -march=$(shell echo $(riscv-march-y) | sed -E 's/(rv32ima|rv64ima)fd([^v_]*)v?/\1\2/')
--
2.43.2
zicfiss / zicfilp introduces a new exception to priv isa `software check
exception` with cause code = 18. This patch implements software check
exception.
If sw check exception was triggered while in usermode, unknown trap is
triggered for usermode. If sw check exception was triggered for kernel
mode, kernel dies.
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/include/asm/asm-prototypes.h | 1 +
arch/riscv/kernel/entry.S | 3 +++
arch/riscv/kernel/traps.c | 20 ++++++++++++++++++++
3 files changed, 24 insertions(+)
diff --git a/arch/riscv/include/asm/asm-prototypes.h b/arch/riscv/include/asm/asm-prototypes.h
index cd627ec289f1..5a27cefd7805 100644
--- a/arch/riscv/include/asm/asm-prototypes.h
+++ b/arch/riscv/include/asm/asm-prototypes.h
@@ -51,6 +51,7 @@ DECLARE_DO_ERROR_INFO(do_trap_ecall_u);
DECLARE_DO_ERROR_INFO(do_trap_ecall_s);
DECLARE_DO_ERROR_INFO(do_trap_ecall_m);
DECLARE_DO_ERROR_INFO(do_trap_break);
+DECLARE_DO_ERROR_INFO(do_trap_software_check);
asmlinkage void handle_bad_stack(struct pt_regs *regs);
asmlinkage void do_page_fault(struct pt_regs *regs);
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 0262b46ab064..89aeae803702 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -353,6 +353,9 @@ SYM_DATA_START_LOCAL(excp_vect_table)
RISCV_PTR do_page_fault /* load page fault */
RISCV_PTR do_trap_unknown
RISCV_PTR do_page_fault /* store page fault */
+ RISCV_PTR do_trap_unknown /* cause=16 */
+ RISCV_PTR do_trap_unknown /* cause=17 */
+ RISCV_PTR do_trap_software_check /* cause=18 is sw check exception */
SYM_DATA_END_LABEL(excp_vect_table, SYM_L_LOCAL, excp_vect_table_end)
#ifndef CONFIG_MMU
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 05a16b1f0aee..b464355f62b2 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -354,6 +354,26 @@ void do_trap_ecall_u(struct pt_regs *regs)
}
+/*
+ * software check exception is defined with risc-v cfi spec. Software check
+ * exception is raised when:-
+ * a) An indirect branch doesn't land on 4 byte aligned PC or `lpad`
+ * instruction or `label` value programmed in `lpad` instr doesn't
+ * match with value setup in `x7`. reported code in `xtval` is 2.
+ * b) `sspopchk` instruction finds a mismatch between top of shadow stack (ssp)
+ * and x1/x5. reported code in `xtval` is 3.
+ */
+asmlinkage __visible __trap_section void do_trap_software_check(struct pt_regs *regs)
+{
+ if (user_mode(regs)) {
+ /* deliver unknown trap to usermode */
+ do_trap_unknown(regs);
+ } else {
+ /* sw check exception coming from kernel is a bug in kernel, die */
+ die(regs, "Kernel BUG");
+ }
+}
+
#ifdef CONFIG_MMU
asmlinkage __visible noinstr void do_page_fault(struct pt_regs *regs)
{
--
2.43.2
This patch enables kernel shadow stack and landing pad enforcement by
invoking a SBI call. As of now it just issues a SBI_EXT_BASE and a hacked
up opensbi implementation sets the LPE/SSE bits in menvcfg
Eventually, we should have fwft [1] interface using which kernel should be
able to set this enforcement properly
[1] - https://lists.riscv.org/g/tech-prs/message/833
Signed-off-by: Deepak Gupta <[email protected]>
---
arch/riscv/kernel/head.S | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index bc248c137c90..1e5bc7b2ee75 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -164,6 +164,13 @@ secondary_start_sbi:
call relocate_enable_mmu
#endif
call .Lsetup_trap_vector
+ /*
+ * Temp hack to get menvcfg.SSE=1 and menvcfg.LPE=1 by invoking
+ * SBI_EXT_BASE
+ */
+ li a6, 0
+ li a7, 0x10
+ ecall
scs_load_current t0
lui t2, 0x1
tail smp_callin
@@ -313,6 +320,13 @@ SYM_CODE_START(_start_kernel)
la tp, init_task
la sp, init_thread_union + THREAD_SIZE
addi sp, sp, -PT_SIZE_ON_STACK
+ /*
+ * Temp hack to get menvcfg.SSE=1 and menvcfg.LPE=1 by invoking
+ * SBI_EXT_BASE
+ */
+ li a6, 0
+ li a7, 0x10
+ ecall
scs_load_current t0
#ifdef CONFIG_KASAN
--
2.43.2
Hi Deepak,
Thanks for the patches!
On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <[email protected]> wrote:
>
> Adding support for dynamic shadow call stack on riscv. zicfiss ISA extn.
> enables protection for shadow stack against stray writes. This patch
> enables scs_* macros to use zicfiss shadow stack pointer (CSR_SSP) instead
> of relying on `gp`.
CONFIG_DYNAMIC_SCS implies that runtime patching is used to select
between software SCS and an alternative hardware implementation (in
arm64's case, PAC instead of hardware shadow stacks). I understand
this series is still an RFC, but I didn't see runtime patching
support. Are you planning on implementing this later?
If there's no plan to actually patch between Zicfiss and SCS at
runtime, CONFIG_DYNAMIC_SCS doesn't seem like the appropriate choice
and we might need a separate config option that still allows you to
reuse most of the software SCS code.
Sami
On Thu, Apr 11, 2024 at 5:30 PM Deepak Gupta <[email protected]> wrote:
>
> On Thu, Apr 11, 2024 at 05:05:38PM +0000, Sami Tolvanen wrote:
> >Hi Deepak,
> >
> >Thanks for the patches!
> >
> >On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <[email protected]> wrote:
> >>
> >> Adding support for dynamic shadow call stack on riscv. zicfiss ISA extn.
> >> enables protection for shadow stack against stray writes. This patch
> >> enables scs_* macros to use zicfiss shadow stack pointer (CSR_SSP) instead
> >> of relying on `gp`.
> >
> >CONFIG_DYNAMIC_SCS implies that runtime patching is used to select
> >between software SCS and an alternative hardware implementation (in
> >arm64's case, PAC instead of hardware shadow stacks). I understand
> >this series is still an RFC, but I didn't see runtime patching
> >support. Are you planning on implementing this later?
>
> Since I didn't see any example on selecting PAC when `CONFIG_DYNAMIC_SCS`
> is selected. So I had that confusion but wasn't sure. I thought of doing it
> but I don't know how to binary rewrite all the functions. It might be too much.
> So I went ahead with using `CONFIG_DYNAMIC_SCS` in this RFC series.
>
> Question:
> If arm64 were to use PAC with CONFIG_DYNAMIC_SCS, how would it fixup the code
> sequences already setup by compiler for shadow stack push and pop in runtime?
> You expect this to be some offline process using some object editing tool or
> a runtime decision?
We use unwind tables for locating instructions to patch, look for
UNWIND_PATCH_PAC_INTO_SCS. The actual patching code is in
arch/arm64/kernel/pi/patch-scs.c. I suspect this is going to be a bit
trickier when patching between two shadow stack options though.
> >If there's no plan to actually patch between Zicfiss and SCS at
> >runtime, CONFIG_DYNAMIC_SCS doesn't seem like the appropriate choice
> >and we might need a separate config option that still allows you to
> >reuse most of the software SCS code.
>
> I wanted to avoid "#ifdef RISCV_SPECIFIC_HW_SHSTK" in arch agnostic scs code.
> And that's why went with CONFIG_DYNAMIC_SCS which sets dynamic static key once.
> And then I use `is_dynamic` everywhere else in arch agnostic scs code.
We could define arch_ functions for any architecture-specific code
(with a weak default implementation), and maybe add a config option
for specifying which way the shadow stack grows?
Sami
On Thu, Apr 11, 2024 at 05:05:38PM +0000, Sami Tolvanen wrote:
>Hi Deepak,
>
>Thanks for the patches!
>
>On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <[email protected]> wrote:
>>
>> Adding support for dynamic shadow call stack on riscv. zicfiss ISA extn.
>> enables protection for shadow stack against stray writes. This patch
>> enables scs_* macros to use zicfiss shadow stack pointer (CSR_SSP) instead
>> of relying on `gp`.
>
>CONFIG_DYNAMIC_SCS implies that runtime patching is used to select
>between software SCS and an alternative hardware implementation (in
>arm64's case, PAC instead of hardware shadow stacks). I understand
>this series is still an RFC, but I didn't see runtime patching
>support. Are you planning on implementing this later?
Since I didn't see any example on selecting PAC when `CONFIG_DYNAMIC_SCS`
is selected. So I had that confusion but wasn't sure. I thought of doing it
but I don't know how to binary rewrite all the functions. It might be too much.
So I went ahead with using `CONFIG_DYNAMIC_SCS` in this RFC series.
Question:
If arm64 were to use PAC with CONFIG_DYNAMIC_SCS, how would it fixup the code
sequences already setup by compiler for shadow stack push and pop in runtime?
You expect this to be some offline process using some object editing tool or
a runtime decision?
>
>If there's no plan to actually patch between Zicfiss and SCS at
>runtime, CONFIG_DYNAMIC_SCS doesn't seem like the appropriate choice
>and we might need a separate config option that still allows you to
>reuse most of the software SCS code.
I wanted to avoid "#ifdef RISCV_SPECIFIC_HW_SHSTK" in arch agnostic scs code.
And that's why went with CONFIG_DYNAMIC_SCS which sets dynamic static key once.
And then I use `is_dynamic` everywhere else in arch agnostic scs code.
>
>Sami
On 09/04/2024 08:10, Deepak Gupta wrote:
> Under CONFIG_SHADOW_CALL_STACK, shadow call stack goes into data section.
> Although with CONFIG_DYNAMIC_SCS on riscv, hardware assisted shadow stack
> are used. Hardware assisted shadow stack on riscv uses PTE.R=0, PTE.W=1 &
> PTE.X=0 encodings. Without CONFIG_DYNAMIC_SCS, shadow stack for init is
> placed in data section and thus regular read/write encodings are applied
> to it. Although with with CONFIG_DYNAMIC_SCS, they need to go into
> different section. This change places it into `.shadowstack` section.
> As part of this change early boot code (`setup_vm`), applies appropriate
> PTE encodings to shadow call stack for init placed in `.shadowstack`
> section.
>
> Signed-off-by: Deepak Gupta <[email protected]>
> ---
> arch/riscv/include/asm/pgtable.h | 4 ++++
> arch/riscv/include/asm/sections.h | 22 +++++++++++++++++++++
> arch/riscv/include/asm/thread_info.h | 10 ++++++++--
> arch/riscv/kernel/vmlinux.lds.S | 12 ++++++++++++
> arch/riscv/mm/init.c | 29 +++++++++++++++++++++-------
> 5 files changed, 68 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 9f8ea0e33eb1..3409b250390d 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -197,6 +197,10 @@ extern struct pt_alloc_ops pt_ops __initdata;
> #define PAGE_KERNEL_READ_EXEC __pgprot((_PAGE_KERNEL & ~_PAGE_WRITE) \
> | _PAGE_EXEC)
>
> +#ifdef CONFIG_DYNAMIC_SCS
> +#define PAGE_KERNEL_SHADOWSTACK __pgprot(_PAGE_KERNEL & ~(_PAGE_READ | _PAGE_EXEC))
> +#endif
> +
Not sure the ifdefs are necessary here, but I'll let others jump in. We
have a lot of them, so we should try not to add.
> #define PAGE_TABLE __pgprot(_PAGE_TABLE)
>
> #define _PAGE_IOREMAP ((_PAGE_KERNEL & ~_PAGE_MTMASK) | _PAGE_IO)
> diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
> index a393d5035c54..4c4154d0021e 100644
> --- a/arch/riscv/include/asm/sections.h
> +++ b/arch/riscv/include/asm/sections.h
> @@ -14,6 +14,10 @@ extern char __init_data_begin[], __init_data_end[];
> extern char __init_text_begin[], __init_text_end[];
> extern char __alt_start[], __alt_end[];
> extern char __exittext_begin[], __exittext_end[];
> +#ifdef CONFIG_DYNAMIC_SCS
> +extern char __init_shstk_start[], __init_shstk_end[];
> +#endif
> +extern char __end_srodata[];
>
> static inline bool is_va_kernel_text(uintptr_t va)
> {
> @@ -31,4 +35,22 @@ static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
> return va >= start && va < end;
> }
>
> +#ifdef CONFIG_DYNAMIC_SCS
> +static inline bool is_va_init_shadow_stack_early(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_start));
> + uintptr_t end = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_end));
> +
> + return va >= start && va < end;
> +}
> +
> +static inline bool is_va_init_shadow_stack(uintptr_t va)
> +{
> + uintptr_t start = (uintptr_t)(__init_shstk_start);
> + uintptr_t end = (uintptr_t)(__init_shstk_end);
> +
> + return va >= start && va < end;
> +}
> +#endif
You could have used an early flag and have only one function but that's
up to you.
> +
> #endif /* __ASM_SECTIONS_H */
> diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
> index 5d473343634b..7ae28d627f84 100644
> --- a/arch/riscv/include/asm/thread_info.h
> +++ b/arch/riscv/include/asm/thread_info.h
> @@ -63,12 +63,18 @@ struct thread_info {
> };
>
> #ifdef CONFIG_SHADOW_CALL_STACK
> +#ifdef CONFIG_DYNAMIC_SCS
> #define INIT_SCS \
> - .scs_base = init_shadow_call_stack, \
> + .scs_base = init_shadow_call_stack, \
> + .scs_sp = &init_shadow_call_stack[SCS_SIZE / sizeof(long)],
> +#else
> +#define INIT_SCS \
> + .scs_base = init_shadow_call_stack, \
> .scs_sp = init_shadow_call_stack,
> +#endif /* CONFIG_DYNAMIC_SCS */
> #else
> #define INIT_SCS
> -#endif
> +#endif /* CONFIG_SHADOW_CALL_STACK */
>
> /*
> * macros/functions for gaining access to the thread information structure
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
> index 002ca58dd998..cccc51f845ab 100644
> --- a/arch/riscv/kernel/vmlinux.lds.S
> +++ b/arch/riscv/kernel/vmlinux.lds.S
> @@ -126,6 +126,18 @@ SECTIONS
> *(.srodata*)
> }
>
> + . = ALIGN(SECTION_ALIGN);
> + __end_srodata = .;
> +
> +#ifdef CONFIG_DYNAMIC_SCS
> + .shadowstack : AT(ADDR(.shadowstack) - LOAD_OFFSET){
> + __init_shstk_start = .;
> + KEEP(*(.shadowstack..init))
> + . = __init_shstk_start + PAGE_SIZE;
> + __init_shstk_end = .;
> + }
> +#endif
> +
> . = ALIGN(SECTION_ALIGN);
> _data = .;
>
> diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> index fe8e159394d8..5b6f0cfa5719 100644
> --- a/arch/riscv/mm/init.c
> +++ b/arch/riscv/mm/init.c
> @@ -713,14 +713,22 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
> if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
> return PAGE_KERNEL_READ;
>
> +#ifdef CONFIG_DYNAMIC_SCS
> + /* If init task's shadow stack va, return write only page protections */
> + if (IS_ENABLED(CONFIG_64BIT) && is_va_init_shadow_stack(va)) {
> + pr_info("Shadow stack protections are being applied to for init\n");
> + return PAGE_KERNEL_SHADOWSTACK;
> + }
> +#endif
To avoid the ifdef here, I would hide it inis_va_init_shadow_stack().
> +
> return PAGE_KERNEL;
> }
>
> void mark_rodata_ro(void)
> {
> - set_kernel_memory(__start_rodata, _data, set_memory_ro);
> + set_kernel_memory(__start_rodata, __end_srodata, set_memory_ro);
> if (IS_ENABLED(CONFIG_64BIT))
> - set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
> + set_kernel_memory(lm_alias(__start_rodata), lm_alias(__end_srodata),
> set_memory_ro);
> }
> #else
> @@ -913,14 +921,21 @@ static void __init create_kernel_page_table(pgd_t *pgdir,
> static void __init create_kernel_page_table(pgd_t *pgdir, bool early)
> {
> uintptr_t va, end_va;
> + pgprot_t prot;
>
> end_va = kernel_map.virt_addr + kernel_map.size;
> - for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE)
> + for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE) {
> + prot = PAGE_KERNEL_EXEC;
> +#ifdef CONFIG_DYNAMIC_SCS
> + if (early && is_va_init_shadow_stack_early(va))
> + prot = PAGE_KERNEL_SHADOWSTACK;
> +#endif
Ditto here to avoid the ifdef, hide it intois_va_init_shadow_stack_early().
> create_pgd_mapping(pgdir, va,
> - kernel_map.phys_addr + (va - kernel_map.virt_addr),
> - PMD_SIZE,
> - early ?
> - PAGE_KERNEL_EXEC : pgprot_from_va(va));
> + kernel_map.phys_addr + (va - kernel_map.virt_addr),
> + PMD_SIZE,
> + early ?
The 3 lines above are not modified, so no need to indent them.
> + prot : pgprot_from_va(va));
> + }
> }
> #endif
>
Apart from the nits above, you can add:
Reviewed-by: Alexandre Ghiti <[email protected]>
Thanks,
Alex
Thanks Alex.
On Sun, May 12, 2024 at 10:12:33PM +0200, Alexandre Ghiti wrote:
>
>On 09/04/2024 08:10, Deepak Gupta wrote:
>>Under CONFIG_SHADOW_CALL_STACK, shadow call stack goes into data section.
>>Although with CONFIG_DYNAMIC_SCS on riscv, hardware assisted shadow stack
>>are used. Hardware assisted shadow stack on riscv uses PTE.R=0, PTE.W=1 &
>>PTE.X=0 encodings. Without CONFIG_DYNAMIC_SCS, shadow stack for init is
>>placed in data section and thus regular read/write encodings are applied
>>to it. Although with with CONFIG_DYNAMIC_SCS, they need to go into
>>different section. This change places it into `.shadowstack` section.
>>As part of this change early boot code (`setup_vm`), applies appropriate
>>PTE encodings to shadow call stack for init placed in `.shadowstack`
>>section.
>>
>>Signed-off-by: Deepak Gupta <[email protected]>
>>---
>> arch/riscv/include/asm/pgtable.h | 4 ++++
>> arch/riscv/include/asm/sections.h | 22 +++++++++++++++++++++
>> arch/riscv/include/asm/thread_info.h | 10 ++++++++--
>> arch/riscv/kernel/vmlinux.lds.S | 12 ++++++++++++
>> arch/riscv/mm/init.c | 29 +++++++++++++++++++++-------
>> 5 files changed, 68 insertions(+), 9 deletions(-)
>>
>>diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>>index 9f8ea0e33eb1..3409b250390d 100644
>>--- a/arch/riscv/include/asm/pgtable.h
>>+++ b/arch/riscv/include/asm/pgtable.h
>>@@ -197,6 +197,10 @@ extern struct pt_alloc_ops pt_ops __initdata;
>> #define PAGE_KERNEL_READ_EXEC __pgprot((_PAGE_KERNEL & ~_PAGE_WRITE) \
>> | _PAGE_EXEC)
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+#define PAGE_KERNEL_SHADOWSTACK __pgprot(_PAGE_KERNEL & ~(_PAGE_READ | _PAGE_EXEC))
>>+#endif
>>+
>
>
>Not sure the ifdefs are necessary here, but I'll let others jump in.
>We have a lot of them, so we should try not to add.
I have no hard leanings either way. I was trying to make sure compile fails if shadow stack
is not enabled. But there are other places where config selection makes sure of this.
So may be not needed here.
>
>
>> #define PAGE_TABLE __pgprot(_PAGE_TABLE)
>> #define _PAGE_IOREMAP ((_PAGE_KERNEL & ~_PAGE_MTMASK) | _PAGE_IO)
>>diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h
>>index a393d5035c54..4c4154d0021e 100644
>>--- a/arch/riscv/include/asm/sections.h
>>+++ b/arch/riscv/include/asm/sections.h
>>@@ -14,6 +14,10 @@ extern char __init_data_begin[], __init_data_end[];
>> extern char __init_text_begin[], __init_text_end[];
>> extern char __alt_start[], __alt_end[];
>> extern char __exittext_begin[], __exittext_end[];
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+extern char __init_shstk_start[], __init_shstk_end[];
>>+#endif
>>+extern char __end_srodata[];
>> static inline bool is_va_kernel_text(uintptr_t va)
>> {
>>@@ -31,4 +35,22 @@ static inline bool is_va_kernel_lm_alias_text(uintptr_t va)
>> return va >= start && va < end;
>> }
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+static inline bool is_va_init_shadow_stack_early(uintptr_t va)
>>+{
>>+ uintptr_t start = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_start));
>>+ uintptr_t end = (uintptr_t)(kernel_mapping_pa_to_va(__init_shstk_end));
>>+
>>+ return va >= start && va < end;
>>+}
>>+
>>+static inline bool is_va_init_shadow_stack(uintptr_t va)
>>+{
>>+ uintptr_t start = (uintptr_t)(__init_shstk_start);
>>+ uintptr_t end = (uintptr_t)(__init_shstk_end);
>>+
>>+ return va >= start && va < end;
>>+}
>>+#endif
>
>
>You could have used an early flag and have only one function but
>that's up to you.
Make sense, yeah I'll do that.
>
>
>>+
>> #endif /* __ASM_SECTIONS_H */
>>diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
>>index 5d473343634b..7ae28d627f84 100644
>>--- a/arch/riscv/include/asm/thread_info.h
>>+++ b/arch/riscv/include/asm/thread_info.h
>>@@ -63,12 +63,18 @@ struct thread_info {
>> };
>> #ifdef CONFIG_SHADOW_CALL_STACK
>>+#ifdef CONFIG_DYNAMIC_SCS
>> #define INIT_SCS \
>>- .scs_base = init_shadow_call_stack, \
>>+ .scs_base = init_shadow_call_stack, \
>>+ .scs_sp = &init_shadow_call_stack[SCS_SIZE / sizeof(long)],
>>+#else
>>+#define INIT_SCS \
>>+ .scs_base = init_shadow_call_stack, \
>> .scs_sp = init_shadow_call_stack,
>>+#endif /* CONFIG_DYNAMIC_SCS */
>> #else
>> #define INIT_SCS
>>-#endif
>>+#endif /* CONFIG_SHADOW_CALL_STACK */
>> /*
>> * macros/functions for gaining access to the thread information structure
>>diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S
>>index 002ca58dd998..cccc51f845ab 100644
>>--- a/arch/riscv/kernel/vmlinux.lds.S
>>+++ b/arch/riscv/kernel/vmlinux.lds.S
>>@@ -126,6 +126,18 @@ SECTIONS
>> *(.srodata*)
>> }
>>+ . = ALIGN(SECTION_ALIGN);
>>+ __end_srodata = .;
>>+
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+ .shadowstack : AT(ADDR(.shadowstack) - LOAD_OFFSET){
>>+ __init_shstk_start = .;
>>+ KEEP(*(.shadowstack..init))
>>+ . = __init_shstk_start + PAGE_SIZE;
>>+ __init_shstk_end = .;
>>+ }
>>+#endif
>>+
>> . = ALIGN(SECTION_ALIGN);
>> _data = .;
>>diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
>>index fe8e159394d8..5b6f0cfa5719 100644
>>--- a/arch/riscv/mm/init.c
>>+++ b/arch/riscv/mm/init.c
>>@@ -713,14 +713,22 @@ static __init pgprot_t pgprot_from_va(uintptr_t va)
>> if (IS_ENABLED(CONFIG_64BIT) && is_va_kernel_lm_alias_text(va))
>> return PAGE_KERNEL_READ;
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+ /* If init task's shadow stack va, return write only page protections */
>>+ if (IS_ENABLED(CONFIG_64BIT) && is_va_init_shadow_stack(va)) {
>>+ pr_info("Shadow stack protections are being applied to for init\n");
>>+ return PAGE_KERNEL_SHADOWSTACK;
>>+ }
>>+#endif
>
>
>To avoid the ifdef here, I would hide it inis_va_init_shadow_stack().
Make sense too.
>
>
>>+
>> return PAGE_KERNEL;
>> }
>> void mark_rodata_ro(void)
>> {
>>- set_kernel_memory(__start_rodata, _data, set_memory_ro);
>>+ set_kernel_memory(__start_rodata, __end_srodata, set_memory_ro);
>> if (IS_ENABLED(CONFIG_64BIT))
>>- set_kernel_memory(lm_alias(__start_rodata), lm_alias(_data),
>>+ set_kernel_memory(lm_alias(__start_rodata), lm_alias(__end_srodata),
>> set_memory_ro);
>> }
>> #else
>>@@ -913,14 +921,21 @@ static void __init create_kernel_page_table(pgd_t *pgdir,
>> static void __init create_kernel_page_table(pgd_t *pgdir, bool early)
>> {
>> uintptr_t va, end_va;
>>+ pgprot_t prot;
>> end_va = kernel_map.virt_addr + kernel_map.size;
>>- for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE)
>>+ for (va = kernel_map.virt_addr; va < end_va; va += PMD_SIZE) {
>>+ prot = PAGE_KERNEL_EXEC;
>>+#ifdef CONFIG_DYNAMIC_SCS
>>+ if (early && is_va_init_shadow_stack_early(va))
>>+ prot = PAGE_KERNEL_SHADOWSTACK;
>>+#endif
>
>
>Ditto here to avoid the ifdef, hide it intois_va_init_shadow_stack_early().
Yes, will do.
>
>
>> create_pgd_mapping(pgdir, va,
>>- kernel_map.phys_addr + (va - kernel_map.virt_addr),
>>- PMD_SIZE,
>>- early ?
>>- PAGE_KERNEL_EXEC : pgprot_from_va(va));
>>+ kernel_map.phys_addr + (va - kernel_map.virt_addr),
>>+ PMD_SIZE,
>>+ early ?
>
>
>The 3 lines above are not modified, so no need to indent them.
noted.
>
>
>>+ prot : pgprot_from_va(va));
>>+ }
>> }
>> #endif
>
>
>Apart from the nits above, you can add:
>
>Reviewed-by: Alexandre Ghiti <[email protected]>
>
>Thanks,
>
>Alex
>