| Message ID | 20240409061043.3269676-9-debug@rivosinc.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC,01/12] riscv: zicfiss / zicfilp extension csr and bit definitions | expand |
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
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
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
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
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(-)