[RFC PATCH v1] riscv kernel control flow integrity

[RFC PATCH v1] riscv kernel control flow integrity

[Deepak Gupta]

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/20240403234054.2020347-1-debug@rivosinc.com/ 
[3] - https://lore.kernel.org/all/20230927224757.1154247-8-samitolvanen@google.com/



_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 01/12] riscv: zicfiss / zicfilp extension csr and bit definitions

[Deepak Gupta]

zicfiss and zicfilp extension gets enabled via b3 and b2 in *envcfg CSR.
menvcfg controls enabling for S/HS mode. henvcfg control enabling for VS
while senvcfg controls enabling for U/VU mode.

zicfilp extension extends *status CSR to hold `expected landing pad` bit.
A trap or interrupt can occur between an indirect jmp/call and target
instr. `expected landing pad` bit from CPU is recorded into xstatus CSR so
that when supervisor performs xret, `expected landing pad` state of CPU can
be restored.

zicfiss adds one new CSR
- CSR_SSP: CSR_SSP contains current shadow stack pointer.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 arch/riscv/include/asm/csr.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 2468c55933cd..9f2b2722b67c 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -18,6 +18,15 @@
 #define SR_MPP		_AC(0x00001800, UL) /* Previously Machine */
 #define SR_SUM		_AC(0x00040000, UL) /* Supervisor User Memory Access */
 
+/* zicfilp landing pad status bit */
+#define SR_SPELP	_AC(0x00800000, UL)
+#define SR_MPELP	_AC(0x020000000000, UL)
+#ifdef CONFIG_RISCV_M_MODE
+#define SR_ELP		SR_MPELP
+#else
+#define SR_ELP		SR_SPELP
+#endif
+
 #define SR_FS		_AC(0x00006000, UL) /* Floating-point Status */
 #define SR_FS_OFF	_AC(0x00000000, UL)
 #define SR_FS_INITIAL	_AC(0x00002000, UL)
@@ -196,6 +205,8 @@
 #define ENVCFG_PBMTE			(_AC(1, ULL) << 62)
 #define ENVCFG_CBZE			(_AC(1, UL) << 7)
 #define ENVCFG_CBCFE			(_AC(1, UL) << 6)
+#define ENVCFG_LPE			(_AC(1, UL) << 2)
+#define ENVCFG_SSE			(_AC(1, UL) << 3)
 #define ENVCFG_CBIE_SHIFT		4
 #define ENVCFG_CBIE			(_AC(0x3, UL) << ENVCFG_CBIE_SHIFT)
 #define ENVCFG_CBIE_ILL			_AC(0x0, UL)
@@ -214,6 +225,11 @@
 #define SMSTATEEN0_HSENVCFG		(_ULL(1) << SMSTATEEN0_HSENVCFG_SHIFT)
 #define SMSTATEEN0_SSTATEEN0_SHIFT	63
 #define SMSTATEEN0_SSTATEEN0		(_ULL(1) << SMSTATEEN0_SSTATEEN0_SHIFT)
+/*
+ * zicfiss user mode csr
+ * CSR_SSP holds current shadow stack pointer.
+ */
+#define CSR_SSP                 0x011
 
 /* symbolic CSR names: */
 #define CSR_CYCLE		0xc00
-- 
2.43.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 02/12] riscv: add landing pad for asm routines.

[Deepak Gupta]

SYM_* macros are used to define assembly routines. In this patch series,
re-define those macros in risc-v arch specific include file to include
a landing pad instruction at the beginning. This is done only when the
compiler flag for landing pad is enabled (i.e. __riscv_zicfilp).

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 arch/riscv/include/asm/linkage.h | 42 ++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/arch/riscv/include/asm/linkage.h b/arch/riscv/include/asm/linkage.h
index 9e88ba23cd2b..bb43ae7dadeb 100644
--- a/arch/riscv/include/asm/linkage.h
+++ b/arch/riscv/include/asm/linkage.h
@@ -6,7 +6,49 @@
 #ifndef _ASM_RISCV_LINKAGE_H
 #define _ASM_RISCV_LINKAGE_H
 
+#ifdef __ASSEMBLY__
+#include <asm/assembler.h>
+#endif
+
 #define __ALIGN		.balign 4
 #define __ALIGN_STR	".balign 4"
 
+#ifdef __riscv_zicfilp
+/*
+ * A landing pad instruction is needed at start of asm routines
+ * re-define macros for asm routines to have a landing pad at
+ * the beginning of function. Currently use label value of 0x1.
+ * Eventually, label should be calculated as a hash over function
+ * signature.
+ */
+#define SYM_FUNC_START(name)				\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)	\
+	lpad 0x1;
+
+#define SYM_FUNC_START_NOALIGN(name)			\
+	SYM_START(name, SYM_L_GLOBAL, SYM_A_NONE)	\
+	lpad 0x1;
+
+#define SYM_FUNC_START_LOCAL(name)			\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_ALIGN)	\
+	lpad 0x1;
+
+#define SYM_FUNC_START_LOCAL_NOALIGN(name)		\
+	SYM_START(name, SYM_L_LOCAL, SYM_A_NONE)	\
+	lpad 0x1;
+
+#define SYM_FUNC_START_WEAK(name)			\
+	SYM_START(name, SYM_L_WEAK, SYM_A_ALIGN)	\
+	lpad 0x1;
+
+#define SYM_FUNC_START_WEAK_NOALIGN(name)		\
+	SYM_START(name, SYM_L_WEAK, SYM_A_NONE)		\
+	lpad 0x1;
+
+#define SYM_TYPED_FUNC_START(name)				\
+	SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)	\
+	lpad 0x1;
+
+#endif
+
 #endif /* _ASM_RISCV_LINKAGE_H */
-- 
2.43.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 03/12] riscv: after saving expected landing pad (elp), clear elp state

[Deepak Gupta]

On trap entry, save expected landing pad state and subsequently clear it
in sstatus so that if there are traps later on in kernel and sret happens
back to same mode, cpu will start faulting.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 arch/riscv/kernel/entry.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 68a24cf9481a..be07355b9eff 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -56,7 +56,7 @@ SYM_CODE_START(handle_exception)
 	 * Disable the FPU/Vector to detect illegal usage of floating point
 	 * or vector in kernel space.
 	 */
-	li t0, SR_SUM | SR_FS_VS
+	li t0, SR_SUM | SR_FS_VS | SR_ELP
 
 	REG_L s0, TASK_TI_USER_SP(tp)
 	csrrc s1, CSR_STATUS, t0
-- 
2.43.2


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 04/12] riscv: update asm call sites with label setup

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 05/12] riscv: fix certain indirect jumps for kernel cfi

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 06/12] scs: place init shadow stack in .shadowstack section

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 07/12] riscv/mm: prepare shadow stack for init task for kernel cfi

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 08/12] riscv: dynamic (zicfiss) shadow call stack support

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 09/12] scs: kernel shadow stack with hardware assistance

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 10/12] riscv/traps: Introduce software check exception

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 11/12] riscv: Kconfig & Makefile for riscv kernel control flow integrity

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[RFC PATCH 12/12] riscv: enable kernel shadow stack and landing pad enforcement

[Deepak Gupta]

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 <debug@rivosinc.com>
---
 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


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 08/12] riscv: dynamic (zicfiss) shadow call stack support

[Sami Tolvanen]

Hi Deepak,

Thanks for the patches!

On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <debug@rivosinc.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 02/12] riscv: add landing pad for asm routines.

[Sami Tolvanen]

On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <debug@rivosinc.com> wrote:
>
> SYM_* macros are used to define assembly routines. In this patch series,
> re-define those macros in risc-v arch specific include file to include
> a landing pad instruction at the beginning. This is done only when the
> compiler flag for landing pad is enabled (i.e. __riscv_zicfilp).
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>  arch/riscv/include/asm/linkage.h | 42 ++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>
> diff --git a/arch/riscv/include/asm/linkage.h b/arch/riscv/include/asm/linkage.h
> index 9e88ba23cd2b..bb43ae7dadeb 100644
> --- a/arch/riscv/include/asm/linkage.h
> +++ b/arch/riscv/include/asm/linkage.h
> @@ -6,7 +6,49 @@
>  #ifndef _ASM_RISCV_LINKAGE_H
>  #define _ASM_RISCV_LINKAGE_H
>
> +#ifdef __ASSEMBLY__
> +#include <asm/assembler.h>
> +#endif
> +
>  #define __ALIGN                .balign 4
>  #define __ALIGN_STR    ".balign 4"
>
> +#ifdef __riscv_zicfilp
> +/*
> + * A landing pad instruction is needed at start of asm routines
> + * re-define macros for asm routines to have a landing pad at
> + * the beginning of function. Currently use label value of 0x1.
> + * Eventually, label should be calculated as a hash over function
> + * signature.
> + */

I haven't seen the compiler implementation for fine-grained Zicfilp
yet, but in the kernel at least, this would ideally reuse as much of
the KCFI plumbing as possible. For example, since only C code has type
information, we left the type hash computation for the compiler, which
allows assembly functions to just reference the appropriate
__kcfi_typeid_* symbol.

Sami

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 08/12] riscv: dynamic (zicfiss) shadow call stack support

[Deepak Gupta]

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 <debug@rivosinc.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 08/12] riscv: dynamic (zicfiss) shadow call stack support

[Sami Tolvanen]

On Thu, Apr 11, 2024 at 5:30 PM Deepak Gupta <debug@rivosinc.com> 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 <debug@rivosinc.com> 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

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 02/12] riscv: add landing pad for asm routines.

[Deepak Gupta]

On Thu, Apr 11, 2024 at 05:15:17PM +0000, Sami Tolvanen wrote:
>On Tue, Apr 9, 2024 at 6:12 AM Deepak Gupta <debug@rivosinc.com> wrote:
>>
>> SYM_* macros are used to define assembly routines. In this patch series,
>> re-define those macros in risc-v arch specific include file to include
>> a landing pad instruction at the beginning. This is done only when the
>> compiler flag for landing pad is enabled (i.e. __riscv_zicfilp).
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> ---
>>  arch/riscv/include/asm/linkage.h | 42 ++++++++++++++++++++++++++++++++
>>  1 file changed, 42 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/linkage.h b/arch/riscv/include/asm/linkage.h
>> index 9e88ba23cd2b..bb43ae7dadeb 100644
>> --- a/arch/riscv/include/asm/linkage.h
>> +++ b/arch/riscv/include/asm/linkage.h
>> @@ -6,7 +6,49 @@
>>  #ifndef _ASM_RISCV_LINKAGE_H
>>  #define _ASM_RISCV_LINKAGE_H
>>
>> +#ifdef __ASSEMBLY__
>> +#include <asm/assembler.h>
>> +#endif
>> +
>>  #define __ALIGN                .balign 4
>>  #define __ALIGN_STR    ".balign 4"
>>
>> +#ifdef __riscv_zicfilp
>> +/*
>> + * A landing pad instruction is needed at start of asm routines
>> + * re-define macros for asm routines to have a landing pad at
>> + * the beginning of function. Currently use label value of 0x1.
>> + * Eventually, label should be calculated as a hash over function
>> + * signature.
>> + */
>
>I haven't seen the compiler implementation for fine-grained Zicfilp
>yet, but in the kernel at least, this would ideally reuse as much of
>the KCFI plumbing as possible. For example, since only C code has type
>information, we left the type hash computation for the compiler, which
>allows assembly functions to just reference the appropriate
>__kcfi_typeid_* symbol.

Fine-grained compiler support hasn't made it in yet.

For reference, compiler that I've been using
https://github.com/sifive/riscv-gnu-toolchain/tree/cfi-dev

Honestly speaking, I didn't realize that kcfi plumbing has made it into
riscv as well. I realized that just after sending the patches.

In principle, I agree it should converge with software based kcfi scheme
as much as possible. However blocker that I see is `hash` is placed just
before function. This breaks for code mapped as execute only scenarios.
And ideally would like to have immediates at callsites instead of loads
(purely perf reason and not security).

But yes in next version, I'll take a look and try to converge as much as
possible.

>
>Sami

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 02/12] riscv: add landing pad for asm routines.

[Sami Tolvanen]

On Thu, Apr 11, 2024 at 5:53 PM Deepak Gupta <debug@rivosinc.com> wrote:
>
> In principle, I agree it should converge with software based kcfi scheme
> as much as possible. However blocker that I see is `hash` is placed just
> before function. This breaks for code mapped as execute only scenarios.
> And ideally would like to have immediates at callsites instead of loads
> (purely perf reason and not security).

I'm not saying the schemes have to be compatible, just that it would
be great to avoid reinventing type annotations etc. For example, when
you implement the fine-grained variant, you could simply override
SYM_TYPED_ENTRY (defined in include/linux/cfi_types.h) to move
__CFI_TYPE inside the function, and then redefine __CFI_TYPE to emit a
landing pad with the correct label. This allows SYM_TYPED_* in
assembly code to work with both KCFI and fine-grained Zicfilp. For the
coarse-grained variant, your current macros are perfectly fine.

> But yes in next version, I'll take a look and try to converge as much as
> possible.

Great, sounds good!

Sami

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 07/12] riscv/mm: prepare shadow stack for init task for kernel cfi

[Alexandre Ghiti]

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 <debug@rivosinc.com>
> ---
>   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 <alexghiti@rivosinc.com>

Thanks,

Alex


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [RFC PATCH 07/12] riscv/mm: prepare shadow stack for init task for kernel cfi

[Deepak Gupta]

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 <debug@rivosinc.com>
>>---
>>  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 <alexghiti@rivosinc.com>
>
>Thanks,
>
>Alex
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv