[RFC PATCH 07/12] riscv/mm: prepare shadow stack for init task for kernel cfi
Alexandre Ghiti
alex at ghiti.fr
Sun May 12 13:12:33 PDT 2024
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 at 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 at rivosinc.com>
Thanks,
Alex
More information about the linux-riscv
mailing list