| Message ID | 20210401002442.2fe56b88@xhacker |
|---|---|
| Headers | show |
| Series | riscv: improve self-protection | expand |
On Wed, Mar 31, 2021 at 10:00 PM Jisheng Zhang <jszhang3@mail.ustc.edu.cn> wrote: > > From: Jisheng Zhang <jszhang@kernel.org> > > They are not needed after booting, so mark them as __init to move them > to the __init section. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > --- > arch/riscv/kernel/traps.c | 2 +- > arch/riscv/mm/init.c | 6 +++--- > arch/riscv/mm/kasan_init.c | 6 +++--- > arch/riscv/mm/ptdump.c | 2 +- > 4 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 1357abf79570..07fdded10c21 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -197,6 +197,6 @@ int is_valid_bugaddr(unsigned long pc) > #endif /* CONFIG_GENERIC_BUG */ > > /* stvec & scratch is already set from head.S */ > -void trap_init(void) > +void __init trap_init(void) > { > } The trap_init() is unused currently so you can drop this change and remove trap_init() as a separate patch. > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > index 067583ab1bd7..76bf2de8aa59 100644 > --- a/arch/riscv/mm/init.c > +++ b/arch/riscv/mm/init.c > @@ -57,7 +57,7 @@ static void __init zone_sizes_init(void) > free_area_init(max_zone_pfns); > } > > -static void setup_zero_page(void) > +static void __init setup_zero_page(void) > { > memset((void *)empty_zero_page, 0, PAGE_SIZE); > } > @@ -75,7 +75,7 @@ static inline void print_mlm(char *name, unsigned long b, unsigned long t) > (((t) - (b)) >> 20)); > } > > -static void print_vm_layout(void) > +static void __init print_vm_layout(void) > { > pr_notice("Virtual kernel memory layout:\n"); > print_mlk("fixmap", (unsigned long)FIXADDR_START, > @@ -557,7 +557,7 @@ static inline void setup_vm_final(void) > #endif /* CONFIG_MMU */ > > #ifdef CONFIG_STRICT_KERNEL_RWX > -void protect_kernel_text_data(void) > +void __init protect_kernel_text_data(void) > { > unsigned long text_start = (unsigned long)_start; > unsigned long init_text_start = (unsigned long)__init_text_begin; > diff --git a/arch/riscv/mm/kasan_init.c b/arch/riscv/mm/kasan_init.c > index 4f85c6d0ddf8..e1d041ac1534 100644 > --- a/arch/riscv/mm/kasan_init.c > +++ b/arch/riscv/mm/kasan_init.c > @@ -60,7 +60,7 @@ asmlinkage void __init kasan_early_init(void) > local_flush_tlb_all(); > } > > -static void kasan_populate_pte(pmd_t *pmd, unsigned long vaddr, unsigned long end) > +static void __init kasan_populate_pte(pmd_t *pmd, unsigned long vaddr, unsigned long end) > { > phys_addr_t phys_addr; > pte_t *ptep, *base_pte; > @@ -82,7 +82,7 @@ static void kasan_populate_pte(pmd_t *pmd, unsigned long vaddr, unsigned long en > set_pmd(pmd, pfn_pmd(PFN_DOWN(__pa(base_pte)), PAGE_TABLE)); > } > > -static void kasan_populate_pmd(pgd_t *pgd, unsigned long vaddr, unsigned long end) > +static void __init kasan_populate_pmd(pgd_t *pgd, unsigned long vaddr, unsigned long end) > { > phys_addr_t phys_addr; > pmd_t *pmdp, *base_pmd; > @@ -117,7 +117,7 @@ static void kasan_populate_pmd(pgd_t *pgd, unsigned long vaddr, unsigned long en > set_pgd(pgd, pfn_pgd(PFN_DOWN(__pa(base_pmd)), PAGE_TABLE)); > } > > -static void kasan_populate_pgd(unsigned long vaddr, unsigned long end) > +static void __init kasan_populate_pgd(unsigned long vaddr, unsigned long end) > { > phys_addr_t phys_addr; > pgd_t *pgdp = pgd_offset_k(vaddr); > diff --git a/arch/riscv/mm/ptdump.c b/arch/riscv/mm/ptdump.c > index ace74dec7492..3b7b6e4d025e 100644 > --- a/arch/riscv/mm/ptdump.c > +++ b/arch/riscv/mm/ptdump.c > @@ -331,7 +331,7 @@ static int ptdump_show(struct seq_file *m, void *v) > > DEFINE_SHOW_ATTRIBUTE(ptdump); > > -static int ptdump_init(void) > +static int __init ptdump_init(void) > { > unsigned int i, j; > > -- > 2.31.0 > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv Apart from above, looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup
On Wed, Mar 31, 2021 at 10:01 PM Jisheng Zhang <jszhang3@mail.ustc.edu.cn> wrote: > > From: Jisheng Zhang <jszhang@kernel.org> > > Constify the sys_call_table so that it will be placed in the .rodata > section. This will cause attempts to modify the table to fail when > strict page permissions are in place. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > arch/riscv/include/asm/syscall.h | 2 +- > arch/riscv/kernel/syscall_table.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h > index 49350c8bd7b0..b933b1583c9f 100644 > --- a/arch/riscv/include/asm/syscall.h > +++ b/arch/riscv/include/asm/syscall.h > @@ -15,7 +15,7 @@ > #include <linux/err.h> > > /* The array of function pointers for syscalls. */ > -extern void *sys_call_table[]; > +extern void * const sys_call_table[]; > > /* > * Only the low 32 bits of orig_r0 are meaningful, so we return int. > diff --git a/arch/riscv/kernel/syscall_table.c b/arch/riscv/kernel/syscall_table.c > index f1ead9df96ca..a63c667c27b3 100644 > --- a/arch/riscv/kernel/syscall_table.c > +++ b/arch/riscv/kernel/syscall_table.c > @@ -13,7 +13,7 @@ > #undef __SYSCALL > #define __SYSCALL(nr, call) [nr] = (call), > > -void *sys_call_table[__NR_syscalls] = { > +void * const sys_call_table[__NR_syscalls] = { > [0 ... __NR_syscalls - 1] = sys_ni_syscall, > #include <asm/unistd.h> > }; > -- > 2.31.0 > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Mar 31, 2021 at 10:05 PM Jisheng Zhang <jszhang3@mail.ustc.edu.cn> wrote: > > From: Jisheng Zhang <jszhang@kernel.org> > > Now we can set ARCH_HAS_STRICT_MODULE_RWX for MMU riscv platforms, this > is good from security perspective. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > arch/riscv/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 87d7b52f278f..9716be3674a2 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -28,6 +28,7 @@ config RISCV > select ARCH_HAS_SET_DIRECT_MAP > select ARCH_HAS_SET_MEMORY > select ARCH_HAS_STRICT_KERNEL_RWX if MMU > + select ARCH_HAS_STRICT_MODULE_RWX if MMU > select ARCH_OPTIONAL_KERNEL_RWX if ARCH_HAS_STRICT_KERNEL_RWX > select ARCH_OPTIONAL_KERNEL_RWX_DEFAULT > select ARCH_WANT_DEFAULT_TOPDOWN_MMAP_LAYOUT if MMU > -- > 2.31.0 > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Mar 31, 2021 at 10:02 PM Jisheng Zhang <jszhang3@mail.ustc.edu.cn> wrote: > > From: Jisheng Zhang <jszhang@kernel.org> > > Allocate PAGE_KERNEL_READ_EXEC(read only, executable) page for kprobes > insn page. This is to prepare for STRICT_MODULE_RWX. > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> Looks good to me. Reviewed-by: Anup Patel <anup@brainfault.org> Regards, Anup > --- > arch/riscv/kernel/probes/kprobes.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/arch/riscv/kernel/probes/kprobes.c b/arch/riscv/kernel/probes/kprobes.c > index 7e2c78e2ca6b..8c1f7a30aeed 100644 > --- a/arch/riscv/kernel/probes/kprobes.c > +++ b/arch/riscv/kernel/probes/kprobes.c > @@ -84,6 +84,14 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p) > return 0; > } > > +void *alloc_insn_page(void) > +{ > + return __vmalloc_node_range(PAGE_SIZE, 1, VMALLOC_START, VMALLOC_END, > + GFP_KERNEL, PAGE_KERNEL_READ_EXEC, > + VM_FLUSH_RESET_PERMS, NUMA_NO_NODE, > + __builtin_return_address(0)); > +} > + > /* install breakpoint in text */ > void __kprobes arch_arm_kprobe(struct kprobe *p) > { > -- > 2.31.0 > > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, 2 Apr 2021 09:38:02 +0530 Anup Patel <anup@brainfault.org> wrote: > > > On Wed, Mar 31, 2021 at 10:00 PM Jisheng Zhang > <jszhang3@mail.ustc.edu.cn> wrote: > > > > From: Jisheng Zhang <jszhang@kernel.org> > > > > They are not needed after booting, so mark them as __init to move them > > to the __init section. > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > --- > > arch/riscv/kernel/traps.c | 2 +- > > arch/riscv/mm/init.c | 6 +++--- > > arch/riscv/mm/kasan_init.c | 6 +++--- > > arch/riscv/mm/ptdump.c | 2 +- > > 4 files changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > > index 1357abf79570..07fdded10c21 100644 > > --- a/arch/riscv/kernel/traps.c > > +++ b/arch/riscv/kernel/traps.c > > @@ -197,6 +197,6 @@ int is_valid_bugaddr(unsigned long pc) > > #endif /* CONFIG_GENERIC_BUG */ > > > > /* stvec & scratch is already set from head.S */ > > -void trap_init(void) > > +void __init trap_init(void) > > { > > } > > The trap_init() is unused currently so you can drop this change > and remove trap_init() as a separate patch. the kernel init/main.c expects a trap_init() implementation in architecture code. Some architecture's implementation is NULL, similar as riscv, for example, arm, powerpc and so on. However I think you are right, the trap_init() can be removed, we need a trivial series to provide a __weak but NULL trap_init() implementation in init/main.c then remove all NULL implementation from all arch. I can take the task to do the clean up. > > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c > > index 067583ab1bd7..76bf2de8aa59 100644 > > --- a/arch/riscv/mm/init.c > > +++ b/arch/riscv/mm/init.c > > @@ -57,7 +57,7 @@ static void __init zone_sizes_init(void) > > free_area_init(max_zone_pfns); > > } > > > > -static void setup_zero_page(void) > > +static void __init setup_zero_page(void) > > { > > memset((void *)empty_zero_page, 0, PAGE_SIZE); I think the zero page is already initialized as "0" because empty_zero_page sits in .bss section. So this setup_zero_page() function can be removed. I will send a newer version later. thanks
From: Jisheng Zhang <jszhang@kernel.org> patch1 is a trivial improvement patch to move some functions to .init section Then following patches improve self-protection by: Marking some variables __ro_after_init Constifing some variables Enabling ARCH_HAS_STRICT_MODULE_RWX Since v1: - no need to move bpf_jit_alloc_exec() and bpf_jit_free_exec() to core because RV32 uses the default module_alloc() for jit code which also meets W^X after patch8 - fix a build error caused by local debug code clean up Jisheng Zhang (9): riscv: add __init section marker to some functions riscv: Mark some global variables __ro_after_init riscv: Constify sys_call_table riscv: Constify sbi_ipi_ops riscv: kprobes: Implement alloc_insn_page() riscv: bpf: Write protect JIT code riscv: bpf: Avoid breaking W^X on RV64 riscv: module: Create module allocations without exec permissions riscv: Set ARCH_HAS_STRICT_MODULE_RWX if MMU arch/riscv/Kconfig | 1 + arch/riscv/include/asm/smp.h | 4 ++-- arch/riscv/include/asm/syscall.h | 2 +- arch/riscv/kernel/module.c | 10 ++++++++-- arch/riscv/kernel/probes/kprobes.c | 8 ++++++++ arch/riscv/kernel/sbi.c | 10 +++++----- arch/riscv/kernel/smp.c | 6 +++--- arch/riscv/kernel/syscall_table.c | 2 +- arch/riscv/kernel/time.c | 2 +- arch/riscv/kernel/traps.c | 2 +- arch/riscv/kernel/vdso.c | 4 ++-- arch/riscv/mm/init.c | 12 ++++++------ arch/riscv/mm/kasan_init.c | 6 +++--- arch/riscv/mm/ptdump.c | 2 +- arch/riscv/net/bpf_jit_comp64.c | 2 +- arch/riscv/net/bpf_jit_core.c | 1 + 16 files changed, 45 insertions(+), 29 deletions(-)