Messages in this thread | | | Date | Mon, 12 Aug 2019 08:10:50 -0700 | From | Christoph Hellwig <> | Subject | Re: [PATCH 2/2] riscv: Add KASAN support |
| |
> 2. KASAN can't debug the modules since the modules are allocated in VMALLOC > area. We mapped the shadow memory, which corresponding to VMALLOC area, > to the kasan_early_shadow_page because we don't have enough physical space > for all the shadow memory corresponding to VMALLOC area.
How do other architectures solve this problem?
> @@ -54,6 +54,8 @@ config RISCV > select EDAC_SUPPORT > select ARCH_HAS_GIGANTIC_PAGE > select ARCH_WANT_HUGE_PMD_SHARE if 64BIT > + select GENERIC_STRNCPY_FROM_USER if KASAN
Is there any reason why we can't always enabled this? Also just enabling the generic efficient strncpy_from_user should probably be a separate patch.
> + select HAVE_ARCH_KASAN if MMU
Based on your cover letter this should be if MMU && 64BIT
> #define __HAVE_ARCH_MEMCPY > extern asmlinkage void *memcpy(void *, const void *, size_t); > +extern asmlinkage void *__memcpy(void *, const void *, size_t); > > #define __HAVE_ARCH_MEMMOVE > extern asmlinkage void *memmove(void *, const void *, size_t); > +extern asmlinkage void *__memmove(void *, const void *, size_t); > + > +#define memcpy(dst, src, len) __memcpy(dst, src, len) > +#define memmove(dst, src, len) __memmove(dst, src, len) > +#define memset(s, c, n) __memset(s, c, n)
This looks weird and at least needs a very good comment. Also with this we effectively don't need the non-prefixed prototypes anymore. Also you probably want to split the renaming of the mem* routines into a separate patch with a proper changelog.
> #include <asm/tlbflush.h> > #include <asm/thread_info.h> > > +#ifdef CONFIG_KASAN > +#include <asm/kasan.h> > +#endif
Any good reason to not just always include the header?
> + > #ifdef CONFIG_DUMMY_CONSOLE > struct screen_info screen_info = { > .orig_video_lines = 30, > @@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p) > > setup_bootmem(); > paging_init(); > + > unflatten_device_tree();
spurious whitespace change.
> diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > index 23cd1a9..9700980 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -46,6 +46,7 @@ SECTIONS > KPROBES_TEXT > ENTRY_TEXT > IRQENTRY_TEXT > + SOFTIRQENTRY_TEXT
Hmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
> +++ b/arch/riscv/mm/kasan_init.c > @@ -0,0 +1,102 @@ > +// SPDX-License-Identifier: GPL-2.0
This probably also wants a copyright statement.
> + // init for swapper_pg_dir
Please use /* */ style comments.
| |