[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190812151050.GJ26897@infradead.org>
Date: Mon, 12 Aug 2019 08:10:50 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Nick Hu <nickhu@...estech.com>
Cc: alankao@...estech.com, paul.walmsley@...ive.com, palmer@...ive.com,
aou@...s.berkeley.edu, green.hu@...il.com, deanbo422@...il.com,
tglx@...utronix.de, linux-riscv@...ts.infradead.org,
linux-kernel@...r.kernel.org, aryabinin@...tuozzo.com,
glider@...gle.com, dvyukov@...gle.com, Anup.Patel@....com,
gregkh@...uxfoundation.org, alexios.zavras@...el.com,
atish.patra@....com, zong@...estech.com, kasan-dev@...glegroups.com
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.
Powered by blists - more mailing lists