* [PATCH v4 1/2] mm: Add PAGE_ALIGN_DOWN macro @ 2022-06-30 8:08 David Gow 2022-06-30 8:08 ` [PATCH v4 2/2] UML: add support for KASAN under x86_64 David Gow 0 siblings, 1 reply; 10+ messages in thread From: David Gow @ 2022-06-30 8:08 UTC (permalink / raw) To: Vincent Whitchurch, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Dmitry Vyukov, Brendan Higgins, Andrew Morton, Andrey Konovalov, Andrey Ryabinin Cc: David Gow, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev This is just the same as PAGE_ALIGN(), but rounds the address down, not up. Suggested-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: David Gow <davidgow@google.com> Acked-by: Andrew Morton <akpm@linux-foundation.org> --- Please take this patch as part of the UML tree, along with patch #2, thanks! No changes to this patch since v3 (just a minor issue with patch #2): https://lore.kernel.org/lkml/20220630074757.2739000-1-davidgow@google.com/ Changes since v2: https://lore.kernel.org/lkml/20220527185600.1236769-1-davidgow@google.com/ - Add Andrew's Acked-by tag. v2 was the first version of this patch (it having been introduced as part of v2 of the UML/KASAN series). There are almost certainly lots of places where this macro should be used: just look for ALIGN_DOWN(..., PAGE_SIZE). I haven't gone through to try to replace them all. --- include/linux/mm.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/linux/mm.h b/include/linux/mm.h index 9f44254af8ce..9abe5975ad11 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -221,6 +221,9 @@ int overcommit_policy_handler(struct ctl_table *, int, void *, size_t *, /* to align the pointer to the (next) page boundary */ #define PAGE_ALIGN(addr) ALIGN(addr, PAGE_SIZE) +/* to align the pointer to the (prev) page boundary */ +#define PAGE_ALIGN_DOWN(addr) ALIGN_DOWN(addr, PAGE_SIZE) + /* test whether an address (unsigned long or pointer) is aligned to PAGE_SIZE */ #define PAGE_ALIGNED(addr) IS_ALIGNED((unsigned long)(addr), PAGE_SIZE) -- 2.37.0.rc0.161.g10f37bed90-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-06-30 8:08 [PATCH v4 1/2] mm: Add PAGE_ALIGN_DOWN macro David Gow @ 2022-06-30 8:08 ` David Gow 2022-06-30 9:41 ` Dmitry Vyukov 0 siblings, 1 reply; 10+ messages in thread From: David Gow @ 2022-06-30 8:08 UTC (permalink / raw) To: Vincent Whitchurch, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Dmitry Vyukov, Brendan Higgins, Andrew Morton, Andrey Konovalov, Andrey Ryabinin Cc: kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev, David Gow From: Patricia Alfonso <trishalfonso@google.com> Make KASAN run on User Mode Linux on x86_64. The UML-specific KASAN initializer uses mmap to map the ~16TB of shadow memory to the location defined by KASAN_SHADOW_OFFSET. kasan_init() utilizes constructors to initialize KASAN before main(). The location of the KASAN shadow memory, starting at KASAN_SHADOW_OFFSET, can be configured using the KASAN_SHADOW_OFFSET option. The default location of this offset is 0x100000000000, which keeps it out-of-the-way even on UML setups with more "physical" memory. For low-memory setups, 0x7fff8000 can be used instead, which fits in an immediate and is therefore faster, as suggested by Dmitry Vyukov. There is usually enough free space at this location; however, it is a config option so that it can be easily changed if needed. Note that, unlike KASAN on other architectures, vmalloc allocations still use the shadow memory allocated upfront, rather than allocating and free-ing it per-vmalloc allocation. If another architecture chooses to go down the same path, we should replace the checks for CONFIG_UML with something more generic, such as: - A CONFIG_KASAN_NO_SHADOW_ALLOC option, which architectures could set - or, a way of having architecture-specific versions of these vmalloc and module shadow memory allocation options. Also note that, while UML supports both KASAN in inline mode (CONFIG_KASAN_INLINE) and static linking (CONFIG_STATIC_LINK), it does not support both at the same time. Signed-off-by: Patricia Alfonso <trishalfonso@google.com> Co-developed-by: Vincent Whitchurch <vincent.whitchurch@axis.com> Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> Signed-off-by: David Gow <davidgow@google.com> Reviewed-by: Johannes Berg <johannes@sipsolutions.net> --- This is v4 of the KASAN/UML port. It should be ready to go, and is identical to v3 module a minor formatting error. Note that this will fail to build if UML is linked statically due to: https://lore.kernel.org/all/20220526185402.955870-1-davidgow@google.com/ Changes since v3: https://lore.kernel.org/lkml/20220630074757.2739000-2-davidgow@google.com/ - Fix some tabs which got converted to spaces by a rogue vim plugin. Changes since v2: https://lore.kernel.org/lkml/20220527185600.1236769-2-davidgow@google.com/ - Don't define CONFIG_KASAN in USER_CFLAGS, given we dont' use it. (Thanks Johannes) - Update patch descriptions and comments given we allocate shadow memory based on the size of the virtual address space, not the "physical" memory used by UML. - This was changed between the original RFC and v1, with KASAN_SHADOW_SIZE's definition being updated. - References to UML using 18TB of space and the shadow memory taking 2.25TB were updated. (Thanks Johannes) - A mention of physical memory in a comment was updated. (Thanks Andrey) - Move some discussion of how the vmalloc() handling could be made more generic from a comment to the commit description. (Thanks Andrey) Changes since RFC v3: https://lore.kernel.org/all/20220526010111.755166-1-davidgow@google.com/ - No longer print "KernelAddressSanitizer initialized" (Johannes) - Document the reason for the CONFIG_UML checks in shadow.c (Dmitry) - Support static builds via kasan_arch_is_ready() (Dmitry) - Get rid of a redundant call to kasam_mem_to_shadow() (Dmitry) - Use PAGE_ALIGN and the new PAGE_ALIGN_DOWN macros (Dmitry) - Reinstate missing arch/um/include/asm/kasan.h file (Johannes) Changes since v1: https://lore.kernel.org/all/20200226004608.8128-1-trishalfonso@google.com/ - Include several fixes from Vincent Whitchurch: https://lore.kernel.org/all/20220525111756.GA15955@axis.com/ - Support for KASAN_VMALLOC, by changing the way kasan_{populate,release}_vmalloc work to update existing shadow memory, rather than allocating anything new. - A similar fix for modules' shadow memory. - Support for KASAN_STACK - This requires the bugfix here: https://lore.kernel.org/lkml/20220523140403.2361040-1-vincent.whitchurch@axis.com/ - Plus a couple of files excluded from KASAN. - Revert the default shadow offset to 0x100000000000 - This was breaking when mem=1G for me, at least. - A few minor fixes to linker sections and scripts. - I've added one to dyn.lds.S on top of the ones Vincent added. --- arch/um/Kconfig | 15 +++++++++++++ arch/um/include/asm/common.lds.S | 2 ++ arch/um/include/asm/kasan.h | 37 ++++++++++++++++++++++++++++++++ arch/um/kernel/Makefile | 3 +++ arch/um/kernel/dyn.lds.S | 6 +++++- arch/um/kernel/mem.c | 19 ++++++++++++++++ arch/um/os-Linux/mem.c | 22 +++++++++++++++++++ arch/um/os-Linux/user_syms.c | 4 ++-- arch/x86/um/Makefile | 3 ++- arch/x86/um/vdso/Makefile | 3 +++ mm/kasan/shadow.c | 29 +++++++++++++++++++++++-- 11 files changed, 137 insertions(+), 6 deletions(-) create mode 100644 arch/um/include/asm/kasan.h diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 8062a0c08952..289c9dc226d6 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -12,6 +12,8 @@ config UML select ARCH_HAS_STRNLEN_USER select ARCH_NO_PREEMPT select HAVE_ARCH_AUDITSYSCALL + select HAVE_ARCH_KASAN if X86_64 + select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN select HAVE_ARCH_SECCOMP_FILTER select HAVE_ASM_MODVERSIONS select HAVE_UID16 @@ -220,6 +222,19 @@ config UML_TIME_TRAVEL_SUPPORT It is safe to say Y, but you probably don't need this. +config KASAN_SHADOW_OFFSET + hex + depends on KASAN + default 0x100000000000 + help + This is the offset at which the ~16TB of shadow memory is + mapped and used by KASAN for memory debugging. This can be any + address that has at least KASAN_SHADOW_SIZE (total address space divided + by 8) amount of space so that the KASAN shadow memory does not conflict + with anything. The default is 0x100000000000, which works even if mem is + set to a large value. On low-memory systems, try 0x7fff8000, as it fits + into the immediate of most instructions, improving performance. + endmenu source "arch/um/drivers/Kconfig" diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S index eca6c452a41b..fd481ac371de 100644 --- a/arch/um/include/asm/common.lds.S +++ b/arch/um/include/asm/common.lds.S @@ -83,6 +83,8 @@ } .init_array : { __init_array_start = .; + *(.kasan_init) + *(.init_array.*) *(.init_array) __init_array_end = .; } diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h new file mode 100644 index 000000000000..0d6547f4ec85 --- /dev/null +++ b/arch/um/include/asm/kasan.h @@ -0,0 +1,37 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_UM_KASAN_H +#define __ASM_UM_KASAN_H + +#include <linux/init.h> +#include <linux/const.h> + +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL) + +/* used in kasan_mem_to_shadow to divide by 8 */ +#define KASAN_SHADOW_SCALE_SHIFT 3 + +#ifdef CONFIG_X86_64 +#define KASAN_HOST_USER_SPACE_END_ADDR 0x00007fffffffffffUL +/* KASAN_SHADOW_SIZE is the size of total address space divided by 8 */ +#define KASAN_SHADOW_SIZE ((KASAN_HOST_USER_SPACE_END_ADDR + 1) >> \ + KASAN_SHADOW_SCALE_SHIFT) +#else +#error "KASAN_SHADOW_SIZE is not defined for this sub-architecture" +#endif /* CONFIG_X86_64 */ + +#define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET) +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) + +#ifdef CONFIG_KASAN +void kasan_init(void); +void kasan_map_memory(void *start, unsigned long len); +extern int kasan_um_is_ready; + +#ifdef CONFIG_STATIC_LINK +#define kasan_arch_is_ready() (kasan_um_is_ready) +#endif +#else +static inline void kasan_init(void) { } +#endif /* CONFIG_KASAN */ + +#endif /* __ASM_UM_KASAN_H */ diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile index 1c2d4b29a3d4..a089217e2f0e 100644 --- a/arch/um/kernel/Makefile +++ b/arch/um/kernel/Makefile @@ -27,6 +27,9 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o obj-$(CONFIG_STACKTRACE) += stacktrace.o obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o +KASAN_SANITIZE_stacktrace.o := n +KASAN_SANITIZE_sysrq.o := n + USER_OBJS := config.o include arch/um/scripts/Makefile.rules diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S index 2f2a8ce92f1e..2b7fc5b54164 100644 --- a/arch/um/kernel/dyn.lds.S +++ b/arch/um/kernel/dyn.lds.S @@ -109,7 +109,11 @@ SECTIONS be empty, which isn't pretty. */ . = ALIGN(32 / 8); .preinit_array : { *(.preinit_array) } - .init_array : { *(.init_array) } + .init_array : { + *(.kasan_init) + *(.init_array.*) + *(.init_array) + } .fini_array : { *(.fini_array) } .data : { INIT_TASK_DATA(KERNEL_STACK_SIZE) diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c index 15295c3237a0..276a1f0b91f1 100644 --- a/arch/um/kernel/mem.c +++ b/arch/um/kernel/mem.c @@ -18,6 +18,25 @@ #include <kern_util.h> #include <mem_user.h> #include <os.h> +#include <linux/sched/task.h> + +#ifdef CONFIG_KASAN +int kasan_um_is_ready; +void kasan_init(void) +{ + /* + * kasan_map_memory will map all of the required address space and + * the host machine will allocate physical memory as necessary. + */ + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); + init_task.kasan_depth = 0; + kasan_um_is_ready = true; +} + +static void (*kasan_init_ptr)(void) +__section(".kasan_init") __used += kasan_init; +#endif /* allocated in paging_init, zeroed in mem_init, and unchanged thereafter */ unsigned long *empty_zero_page = NULL; diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c index 3c1b77474d2d..8530b2e08604 100644 --- a/arch/um/os-Linux/mem.c +++ b/arch/um/os-Linux/mem.c @@ -17,6 +17,28 @@ #include <init.h> #include <os.h> +/* + * kasan_map_memory - maps memory from @start with a size of @len. + * The allocated memory is filled with zeroes upon success. + * @start: the start address of the memory to be mapped + * @len: the length of the memory to be mapped + * + * This function is used to map shadow memory for KASAN in uml + */ +void kasan_map_memory(void *start, size_t len) +{ + if (mmap(start, + len, + PROT_READ|PROT_WRITE, + MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, + -1, + 0) == MAP_FAILED) { + os_info("Couldn't allocate shadow memory: %s\n.", + strerror(errno)); + exit(1); + } +} + /* Set by make_tempfile() during early boot. */ static char *tempdir = NULL; diff --git a/arch/um/os-Linux/user_syms.c b/arch/um/os-Linux/user_syms.c index 715594fe5719..cb667c9225ab 100644 --- a/arch/um/os-Linux/user_syms.c +++ b/arch/um/os-Linux/user_syms.c @@ -27,10 +27,10 @@ EXPORT_SYMBOL(strstr); #ifndef __x86_64__ extern void *memcpy(void *, const void *, size_t); EXPORT_SYMBOL(memcpy); -#endif - EXPORT_SYMBOL(memmove); EXPORT_SYMBOL(memset); +#endif + EXPORT_SYMBOL(printf); /* Here, instead, I can provide a fake prototype. Yes, someone cares: genksyms. diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile index ba5789c35809..f778e37494ba 100644 --- a/arch/x86/um/Makefile +++ b/arch/x86/um/Makefile @@ -28,7 +28,8 @@ else obj-y += syscalls_64.o vdso/ -subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o +subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o \ + ../lib/memmove_64.o ../lib/memset_64.o endif diff --git a/arch/x86/um/vdso/Makefile b/arch/x86/um/vdso/Makefile index 5943387e3f35..8c0396fd0e6f 100644 --- a/arch/x86/um/vdso/Makefile +++ b/arch/x86/um/vdso/Makefile @@ -3,6 +3,9 @@ # Building vDSO images for x86. # +# do not instrument on vdso because KASAN is not compatible with user mode +KASAN_SANITIZE := n + # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in. KCOV_INSTRUMENT := n diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c index a4f07de21771..0e3648b603a6 100644 --- a/mm/kasan/shadow.c +++ b/mm/kasan/shadow.c @@ -295,9 +295,22 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size) return 0; shadow_start = (unsigned long)kasan_mem_to_shadow((void *)addr); - shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE); shadow_end = (unsigned long)kasan_mem_to_shadow((void *)addr + size); - shadow_end = ALIGN(shadow_end, PAGE_SIZE); + + /* + * User Mode Linux maps enough shadow memory for all of virtual memory + * at boot, so doesn't need to allocate more on vmalloc, just clear it. + * + * The remaining CONFIG_UML checks in this file exist for the same + * reason. + */ + if (IS_ENABLED(CONFIG_UML)) { + __memset((void *)shadow_start, KASAN_VMALLOC_INVALID, shadow_end - shadow_start); + return 0; + } + + shadow_start = PAGE_ALIGN_DOWN(shadow_start); + shadow_end = PAGE_ALIGN(shadow_end); ret = apply_to_page_range(&init_mm, shadow_start, shadow_end - shadow_start, @@ -466,6 +479,10 @@ void kasan_release_vmalloc(unsigned long start, unsigned long end, if (shadow_end > shadow_start) { size = shadow_end - shadow_start; + if (IS_ENABLED(CONFIG_UML)) { + __memset(shadow_start, KASAN_SHADOW_INIT, shadow_end - shadow_start); + return; + } apply_to_existing_page_range(&init_mm, (unsigned long)shadow_start, size, kasan_depopulate_vmalloc_pte, @@ -531,6 +548,11 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask) if (WARN_ON(!PAGE_ALIGNED(shadow_start))) return -EINVAL; + if (IS_ENABLED(CONFIG_UML)) { + __memset((void *)shadow_start, KASAN_SHADOW_INIT, shadow_size); + return 0; + } + ret = __vmalloc_node_range(shadow_size, 1, shadow_start, shadow_start + shadow_size, GFP_KERNEL, @@ -554,6 +576,9 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask) void kasan_free_module_shadow(const struct vm_struct *vm) { + if (IS_ENABLED(CONFIG_UML)) + return; + if (vm->flags & VM_KASAN) vfree(kasan_mem_to_shadow(vm->addr)); } -- 2.37.0.rc0.161.g10f37bed90-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-06-30 8:08 ` [PATCH v4 2/2] UML: add support for KASAN under x86_64 David Gow @ 2022-06-30 9:41 ` Dmitry Vyukov 2022-06-30 12:54 ` Vincent Whitchurch 0 siblings, 1 reply; 10+ messages in thread From: Dmitry Vyukov @ 2022-06-30 9:41 UTC (permalink / raw) To: David Gow Cc: Vincent Whitchurch, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Konovalov, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Thu, 30 Jun 2022 at 10:08, David Gow <davidgow@google.com> wrote: > > From: Patricia Alfonso <trishalfonso@google.com> > > Make KASAN run on User Mode Linux on x86_64. > > The UML-specific KASAN initializer uses mmap to map the ~16TB of shadow > memory to the location defined by KASAN_SHADOW_OFFSET. kasan_init() > utilizes constructors to initialize KASAN before main(). > > The location of the KASAN shadow memory, starting at > KASAN_SHADOW_OFFSET, can be configured using the KASAN_SHADOW_OFFSET > option. The default location of this offset is 0x100000000000, which > keeps it out-of-the-way even on UML setups with more "physical" memory. > > For low-memory setups, 0x7fff8000 can be used instead, which fits in an > immediate and is therefore faster, as suggested by Dmitry Vyukov. There > is usually enough free space at this location; however, it is a config > option so that it can be easily changed if needed. > > Note that, unlike KASAN on other architectures, vmalloc allocations > still use the shadow memory allocated upfront, rather than allocating > and free-ing it per-vmalloc allocation. > > If another architecture chooses to go down the same path, we should > replace the checks for CONFIG_UML with something more generic, such > as: > - A CONFIG_KASAN_NO_SHADOW_ALLOC option, which architectures could set > - or, a way of having architecture-specific versions of these vmalloc > and module shadow memory allocation options. > > Also note that, while UML supports both KASAN in inline mode > (CONFIG_KASAN_INLINE) and static linking (CONFIG_STATIC_LINK), it does > not support both at the same time. > > Signed-off-by: Patricia Alfonso <trishalfonso@google.com> > Co-developed-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > Signed-off-by: Vincent Whitchurch <vincent.whitchurch@axis.com> > Signed-off-by: David Gow <davidgow@google.com> > Reviewed-by: Johannes Berg <johannes@sipsolutions.net> > --- > This is v4 of the KASAN/UML port. It should be ready to go, and is > identical to v3 module a minor formatting error. > > Note that this will fail to build if UML is linked statically due to: > https://lore.kernel.org/all/20220526185402.955870-1-davidgow@google.com/ > > Changes since v3: > https://lore.kernel.org/lkml/20220630074757.2739000-2-davidgow@google.com/ > - Fix some tabs which got converted to spaces by a rogue vim plugin. > > Changes since v2: > https://lore.kernel.org/lkml/20220527185600.1236769-2-davidgow@google.com/ > - Don't define CONFIG_KASAN in USER_CFLAGS, given we dont' use it. > (Thanks Johannes) > - Update patch descriptions and comments given we allocate shadow memory based > on the size of the virtual address space, not the "physical" memory > used by UML. > - This was changed between the original RFC and v1, with > KASAN_SHADOW_SIZE's definition being updated. > - References to UML using 18TB of space and the shadow memory taking > 2.25TB were updated. (Thanks Johannes) > - A mention of physical memory in a comment was updated. (Thanks > Andrey) > - Move some discussion of how the vmalloc() handling could be made more > generic from a comment to the commit description. (Thanks Andrey) > > Changes since RFC v3: > https://lore.kernel.org/all/20220526010111.755166-1-davidgow@google.com/ > - No longer print "KernelAddressSanitizer initialized" (Johannes) > - Document the reason for the CONFIG_UML checks in shadow.c (Dmitry) > - Support static builds via kasan_arch_is_ready() (Dmitry) > - Get rid of a redundant call to kasam_mem_to_shadow() (Dmitry) > - Use PAGE_ALIGN and the new PAGE_ALIGN_DOWN macros (Dmitry) > - Reinstate missing arch/um/include/asm/kasan.h file (Johannes) > > Changes since v1: > https://lore.kernel.org/all/20200226004608.8128-1-trishalfonso@google.com/ > - Include several fixes from Vincent Whitchurch: > https://lore.kernel.org/all/20220525111756.GA15955@axis.com/ > - Support for KASAN_VMALLOC, by changing the way > kasan_{populate,release}_vmalloc work to update existing shadow > memory, rather than allocating anything new. > - A similar fix for modules' shadow memory. > - Support for KASAN_STACK > - This requires the bugfix here: > https://lore.kernel.org/lkml/20220523140403.2361040-1-vincent.whitchurch@axis.com/ > - Plus a couple of files excluded from KASAN. > - Revert the default shadow offset to 0x100000000000 > - This was breaking when mem=1G for me, at least. > - A few minor fixes to linker sections and scripts. > - I've added one to dyn.lds.S on top of the ones Vincent added. > > --- > arch/um/Kconfig | 15 +++++++++++++ > arch/um/include/asm/common.lds.S | 2 ++ > arch/um/include/asm/kasan.h | 37 ++++++++++++++++++++++++++++++++ > arch/um/kernel/Makefile | 3 +++ > arch/um/kernel/dyn.lds.S | 6 +++++- > arch/um/kernel/mem.c | 19 ++++++++++++++++ > arch/um/os-Linux/mem.c | 22 +++++++++++++++++++ > arch/um/os-Linux/user_syms.c | 4 ++-- > arch/x86/um/Makefile | 3 ++- > arch/x86/um/vdso/Makefile | 3 +++ > mm/kasan/shadow.c | 29 +++++++++++++++++++++++-- > 11 files changed, 137 insertions(+), 6 deletions(-) > create mode 100644 arch/um/include/asm/kasan.h > > diff --git a/arch/um/Kconfig b/arch/um/Kconfig > index 8062a0c08952..289c9dc226d6 100644 > --- a/arch/um/Kconfig > +++ b/arch/um/Kconfig > @@ -12,6 +12,8 @@ config UML > select ARCH_HAS_STRNLEN_USER > select ARCH_NO_PREEMPT > select HAVE_ARCH_AUDITSYSCALL > + select HAVE_ARCH_KASAN if X86_64 > + select HAVE_ARCH_KASAN_VMALLOC if HAVE_ARCH_KASAN > select HAVE_ARCH_SECCOMP_FILTER > select HAVE_ASM_MODVERSIONS > select HAVE_UID16 > @@ -220,6 +222,19 @@ config UML_TIME_TRAVEL_SUPPORT > > It is safe to say Y, but you probably don't need this. > > +config KASAN_SHADOW_OFFSET > + hex > + depends on KASAN > + default 0x100000000000 > + help > + This is the offset at which the ~16TB of shadow memory is > + mapped and used by KASAN for memory debugging. This can be any > + address that has at least KASAN_SHADOW_SIZE (total address space divided > + by 8) amount of space so that the KASAN shadow memory does not conflict > + with anything. The default is 0x100000000000, which works even if mem is > + set to a large value. On low-memory systems, try 0x7fff8000, as it fits > + into the immediate of most instructions, improving performance. > + > endmenu > > source "arch/um/drivers/Kconfig" > diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S > index eca6c452a41b..fd481ac371de 100644 > --- a/arch/um/include/asm/common.lds.S > +++ b/arch/um/include/asm/common.lds.S > @@ -83,6 +83,8 @@ > } > .init_array : { > __init_array_start = .; > + *(.kasan_init) > + *(.init_array.*) > *(.init_array) > __init_array_end = .; > } > diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h > new file mode 100644 > index 000000000000..0d6547f4ec85 > --- /dev/null > +++ b/arch/um/include/asm/kasan.h > @@ -0,0 +1,37 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __ASM_UM_KASAN_H > +#define __ASM_UM_KASAN_H > + > +#include <linux/init.h> > +#include <linux/const.h> > + > +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL) > + > +/* used in kasan_mem_to_shadow to divide by 8 */ > +#define KASAN_SHADOW_SCALE_SHIFT 3 > + > +#ifdef CONFIG_X86_64 > +#define KASAN_HOST_USER_SPACE_END_ADDR 0x00007fffffffffffUL > +/* KASAN_SHADOW_SIZE is the size of total address space divided by 8 */ > +#define KASAN_SHADOW_SIZE ((KASAN_HOST_USER_SPACE_END_ADDR + 1) >> \ > + KASAN_SHADOW_SCALE_SHIFT) > +#else > +#error "KASAN_SHADOW_SIZE is not defined for this sub-architecture" > +#endif /* CONFIG_X86_64 */ > + > +#define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET) > +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) > + > +#ifdef CONFIG_KASAN > +void kasan_init(void); > +void kasan_map_memory(void *start, unsigned long len); > +extern int kasan_um_is_ready; > + > +#ifdef CONFIG_STATIC_LINK > +#define kasan_arch_is_ready() (kasan_um_is_ready) > +#endif > +#else > +static inline void kasan_init(void) { } > +#endif /* CONFIG_KASAN */ > + > +#endif /* __ASM_UM_KASAN_H */ > diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile > index 1c2d4b29a3d4..a089217e2f0e 100644 > --- a/arch/um/kernel/Makefile > +++ b/arch/um/kernel/Makefile > @@ -27,6 +27,9 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > obj-$(CONFIG_STACKTRACE) += stacktrace.o > obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o > > +KASAN_SANITIZE_stacktrace.o := n > +KASAN_SANITIZE_sysrq.o := n Why are these needed? It's helpful to leave some comments for any of *_SANITIZE:=n. Otherwise later it's unclear if it's due to some latent bugs, some inherent incompatibility, something that can be fixed, etc. Otherwise the patch looks good to me. > + > USER_OBJS := config.o > > include arch/um/scripts/Makefile.rules > diff --git a/arch/um/kernel/dyn.lds.S b/arch/um/kernel/dyn.lds.S > index 2f2a8ce92f1e..2b7fc5b54164 100644 > --- a/arch/um/kernel/dyn.lds.S > +++ b/arch/um/kernel/dyn.lds.S > @@ -109,7 +109,11 @@ SECTIONS > be empty, which isn't pretty. */ > . = ALIGN(32 / 8); > .preinit_array : { *(.preinit_array) } > - .init_array : { *(.init_array) } > + .init_array : { > + *(.kasan_init) > + *(.init_array.*) > + *(.init_array) > + } > .fini_array : { *(.fini_array) } > .data : { > INIT_TASK_DATA(KERNEL_STACK_SIZE) > diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c > index 15295c3237a0..276a1f0b91f1 100644 > --- a/arch/um/kernel/mem.c > +++ b/arch/um/kernel/mem.c > @@ -18,6 +18,25 @@ > #include <kern_util.h> > #include <mem_user.h> > #include <os.h> > +#include <linux/sched/task.h> > + > +#ifdef CONFIG_KASAN > +int kasan_um_is_ready; > +void kasan_init(void) > +{ > + /* > + * kasan_map_memory will map all of the required address space and > + * the host machine will allocate physical memory as necessary. > + */ > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > + init_task.kasan_depth = 0; > + kasan_um_is_ready = true; > +} > + > +static void (*kasan_init_ptr)(void) > +__section(".kasan_init") __used > += kasan_init; > +#endif > > /* allocated in paging_init, zeroed in mem_init, and unchanged thereafter */ > unsigned long *empty_zero_page = NULL; > diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c > index 3c1b77474d2d..8530b2e08604 100644 > --- a/arch/um/os-Linux/mem.c > +++ b/arch/um/os-Linux/mem.c > @@ -17,6 +17,28 @@ > #include <init.h> > #include <os.h> > > +/* > + * kasan_map_memory - maps memory from @start with a size of @len. > + * The allocated memory is filled with zeroes upon success. > + * @start: the start address of the memory to be mapped > + * @len: the length of the memory to be mapped > + * > + * This function is used to map shadow memory for KASAN in uml > + */ > +void kasan_map_memory(void *start, size_t len) > +{ > + if (mmap(start, > + len, > + PROT_READ|PROT_WRITE, > + MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, > + -1, > + 0) == MAP_FAILED) { > + os_info("Couldn't allocate shadow memory: %s\n.", > + strerror(errno)); > + exit(1); > + } > +} > + > /* Set by make_tempfile() during early boot. */ > static char *tempdir = NULL; > > diff --git a/arch/um/os-Linux/user_syms.c b/arch/um/os-Linux/user_syms.c > index 715594fe5719..cb667c9225ab 100644 > --- a/arch/um/os-Linux/user_syms.c > +++ b/arch/um/os-Linux/user_syms.c > @@ -27,10 +27,10 @@ EXPORT_SYMBOL(strstr); > #ifndef __x86_64__ > extern void *memcpy(void *, const void *, size_t); > EXPORT_SYMBOL(memcpy); > -#endif > - > EXPORT_SYMBOL(memmove); > EXPORT_SYMBOL(memset); > +#endif > + > EXPORT_SYMBOL(printf); > > /* Here, instead, I can provide a fake prototype. Yes, someone cares: genksyms. > diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile > index ba5789c35809..f778e37494ba 100644 > --- a/arch/x86/um/Makefile > +++ b/arch/x86/um/Makefile > @@ -28,7 +28,8 @@ else > > obj-y += syscalls_64.o vdso/ > > -subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o > +subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o \ > + ../lib/memmove_64.o ../lib/memset_64.o > > endif > > diff --git a/arch/x86/um/vdso/Makefile b/arch/x86/um/vdso/Makefile > index 5943387e3f35..8c0396fd0e6f 100644 > --- a/arch/x86/um/vdso/Makefile > +++ b/arch/x86/um/vdso/Makefile > @@ -3,6 +3,9 @@ > # Building vDSO images for x86. > # > > +# do not instrument on vdso because KASAN is not compatible with user mode > +KASAN_SANITIZE := n > + > # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in. > KCOV_INSTRUMENT := n > > diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c > index a4f07de21771..0e3648b603a6 100644 > --- a/mm/kasan/shadow.c > +++ b/mm/kasan/shadow.c > @@ -295,9 +295,22 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size) > return 0; > > shadow_start = (unsigned long)kasan_mem_to_shadow((void *)addr); > - shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE); > shadow_end = (unsigned long)kasan_mem_to_shadow((void *)addr + size); > - shadow_end = ALIGN(shadow_end, PAGE_SIZE); > + > + /* > + * User Mode Linux maps enough shadow memory for all of virtual memory > + * at boot, so doesn't need to allocate more on vmalloc, just clear it. > + * > + * The remaining CONFIG_UML checks in this file exist for the same > + * reason. > + */ > + if (IS_ENABLED(CONFIG_UML)) { > + __memset((void *)shadow_start, KASAN_VMALLOC_INVALID, shadow_end - shadow_start); > + return 0; > + } > + > + shadow_start = PAGE_ALIGN_DOWN(shadow_start); > + shadow_end = PAGE_ALIGN(shadow_end); > > ret = apply_to_page_range(&init_mm, shadow_start, > shadow_end - shadow_start, > @@ -466,6 +479,10 @@ void kasan_release_vmalloc(unsigned long start, unsigned long end, > > if (shadow_end > shadow_start) { > size = shadow_end - shadow_start; > + if (IS_ENABLED(CONFIG_UML)) { > + __memset(shadow_start, KASAN_SHADOW_INIT, shadow_end - shadow_start); > + return; > + } > apply_to_existing_page_range(&init_mm, > (unsigned long)shadow_start, > size, kasan_depopulate_vmalloc_pte, > @@ -531,6 +548,11 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask) > if (WARN_ON(!PAGE_ALIGNED(shadow_start))) > return -EINVAL; > > + if (IS_ENABLED(CONFIG_UML)) { > + __memset((void *)shadow_start, KASAN_SHADOW_INIT, shadow_size); > + return 0; > + } > + > ret = __vmalloc_node_range(shadow_size, 1, shadow_start, > shadow_start + shadow_size, > GFP_KERNEL, > @@ -554,6 +576,9 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask) > > void kasan_free_module_shadow(const struct vm_struct *vm) > { > + if (IS_ENABLED(CONFIG_UML)) > + return; > + > if (vm->flags & VM_KASAN) > vfree(kasan_mem_to_shadow(vm->addr)); > } > -- > 2.37.0.rc0.161.g10f37bed90-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-06-30 9:41 ` Dmitry Vyukov @ 2022-06-30 12:54 ` Vincent Whitchurch 2022-06-30 13:28 ` Andrey Konovalov 0 siblings, 1 reply; 10+ messages in thread From: Vincent Whitchurch @ 2022-06-30 12:54 UTC (permalink / raw) To: Dmitry Vyukov Cc: David Gow, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Konovalov, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Thu, Jun 30, 2022 at 11:41:04AM +0200, Dmitry Vyukov wrote: > On Thu, 30 Jun 2022 at 10:08, David Gow <davidgow@google.com> wrote: > > diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile > > index 1c2d4b29a3d4..a089217e2f0e 100644 > > --- a/arch/um/kernel/Makefile > > +++ b/arch/um/kernel/Makefile > > @@ -27,6 +27,9 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > obj-$(CONFIG_STACKTRACE) += stacktrace.o > > obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o > > > > +KASAN_SANITIZE_stacktrace.o := n > > +KASAN_SANITIZE_sysrq.o := n > > Why are these needed? > It's helpful to leave some comments for any of *_SANITIZE:=n. > Otherwise later it's unclear if it's due to some latent bugs, some > inherent incompatibility, something that can be fixed, etc. I believe I saw the stacktrace code itself triggering KASAN splats and causing recursion when sanitization was not disabled on it. I noticed that other architectures disabled sanitization of their stacktrace code, eg. ARM in commit 4d576cab16f57e1f87978f ("ARM: 9028/1: disable KASAN in call stack capturing routines"), so I did not investigate it further. (Note that despite the name, sysrq.c is also just stacktrace code.) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-06-30 12:54 ` Vincent Whitchurch @ 2022-06-30 13:28 ` Andrey Konovalov 2022-07-01 9:08 ` David Gow 0 siblings, 1 reply; 10+ messages in thread From: Andrey Konovalov @ 2022-06-30 13:28 UTC (permalink / raw) To: Vincent Whitchurch, Dmitry Vyukov Cc: David Gow, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Thu, Jun 30, 2022 at 2:54 PM Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > On Thu, Jun 30, 2022 at 11:41:04AM +0200, Dmitry Vyukov wrote: > > On Thu, 30 Jun 2022 at 10:08, David Gow <davidgow@google.com> wrote: > > > diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile > > > index 1c2d4b29a3d4..a089217e2f0e 100644 > > > --- a/arch/um/kernel/Makefile > > > +++ b/arch/um/kernel/Makefile > > > @@ -27,6 +27,9 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > > obj-$(CONFIG_STACKTRACE) += stacktrace.o > > > obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o > > > > > > +KASAN_SANITIZE_stacktrace.o := n > > > +KASAN_SANITIZE_sysrq.o := n > > > > Why are these needed? > > It's helpful to leave some comments for any of *_SANITIZE:=n. > > Otherwise later it's unclear if it's due to some latent bugs, some > > inherent incompatibility, something that can be fixed, etc. > > I believe I saw the stacktrace code itself triggering KASAN splats and > causing recursion when sanitization was not disabled on it. I noticed > that other architectures disabled sanitization of their stacktrace code, > eg. ARM in commit 4d576cab16f57e1f87978f ("ARM: 9028/1: disable KASAN in > call stack capturing routines"), so I did not investigate it further. > > (Note that despite the name, sysrq.c is also just stacktrace code.) Stack trace collection code might trigger KASAN splats when walking stack frames, but this can be resolved by using unchecked accesses. The main reason to disable instrumentation here is for performance reasons, see the upcoming patch for arm64 [1] for some details. [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-06-30 13:28 ` Andrey Konovalov @ 2022-07-01 9:08 ` David Gow 2022-07-01 9:16 ` Vincent Whitchurch 0 siblings, 1 reply; 10+ messages in thread From: David Gow @ 2022-07-01 9:08 UTC (permalink / raw) To: Andrey Konovalov Cc: Vincent Whitchurch, Dmitry Vyukov, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Thu, Jun 30, 2022 at 9:29 PM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > On Thu, Jun 30, 2022 at 2:54 PM Vincent Whitchurch > <vincent.whitchurch@axis.com> wrote: > > > > On Thu, Jun 30, 2022 at 11:41:04AM +0200, Dmitry Vyukov wrote: > > > On Thu, 30 Jun 2022 at 10:08, David Gow <davidgow@google.com> wrote: > > > > diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile > > > > index 1c2d4b29a3d4..a089217e2f0e 100644 > > > > --- a/arch/um/kernel/Makefile > > > > +++ b/arch/um/kernel/Makefile > > > > @@ -27,6 +27,9 @@ obj-$(CONFIG_EARLY_PRINTK) += early_printk.o > > > > obj-$(CONFIG_STACKTRACE) += stacktrace.o > > > > obj-$(CONFIG_GENERIC_PCI_IOMAP) += ioport.o > > > > > > > > +KASAN_SANITIZE_stacktrace.o := n > > > > +KASAN_SANITIZE_sysrq.o := n > > > > > > Why are these needed? > > > It's helpful to leave some comments for any of *_SANITIZE:=n. > > > Otherwise later it's unclear if it's due to some latent bugs, some > > > inherent incompatibility, something that can be fixed, etc. > > > > I believe I saw the stacktrace code itself triggering KASAN splats and > > causing recursion when sanitization was not disabled on it. I noticed > > that other architectures disabled sanitization of their stacktrace code, > > eg. ARM in commit 4d576cab16f57e1f87978f ("ARM: 9028/1: disable KASAN in > > call stack capturing routines"), so I did not investigate it further. > > > > (Note that despite the name, sysrq.c is also just stacktrace code.) > > Stack trace collection code might trigger KASAN splats when walking > stack frames, but this can be resolved by using unchecked accesses. > The main reason to disable instrumentation here is for performance > reasons, see the upcoming patch for arm64 [1] for some details. > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 Ah -- that does it! Using READ_ONCE_NOCHECK() in dump_trace() gets rid of the nasty recursive KASAN failures we were getting in the tests. I'll send out v5 with those files instrumented again. Thanks! -- David ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-07-01 9:08 ` David Gow @ 2022-07-01 9:16 ` Vincent Whitchurch 2022-07-01 9:43 ` David Gow 0 siblings, 1 reply; 10+ messages in thread From: Vincent Whitchurch @ 2022-07-01 9:16 UTC (permalink / raw) To: David Gow Cc: Andrey Konovalov, Dmitry Vyukov, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Fri, Jul 01, 2022 at 11:08:27AM +0200, David Gow wrote: > On Thu, Jun 30, 2022 at 9:29 PM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > Stack trace collection code might trigger KASAN splats when walking > > stack frames, but this can be resolved by using unchecked accesses. > > The main reason to disable instrumentation here is for performance > > reasons, see the upcoming patch for arm64 [1] for some details. > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 > > Ah -- that does it! Using READ_ONCE_NOCHECK() in dump_trace() gets rid > of the nasty recursive KASAN failures we were getting in the tests. > > I'll send out v5 with those files instrumented again. Hmm, do we really want that? In the patch Andrey linked to above he removed the READ_ONCE_NOCHECK() and added the KASAN_SANITIZE on the corresponding files for arm64, just like it's already the case in this patch for UML. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-07-01 9:16 ` Vincent Whitchurch @ 2022-07-01 9:43 ` David Gow 2022-07-01 10:04 ` Vincent Whitchurch 0 siblings, 1 reply; 10+ messages in thread From: David Gow @ 2022-07-01 9:43 UTC (permalink / raw) To: Vincent Whitchurch Cc: Andrey Konovalov, Dmitry Vyukov, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev [-- Attachment #1: Type: text/plain, Size: 2119 bytes --] On Fri, Jul 1, 2022 at 5:16 PM Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > On Fri, Jul 01, 2022 at 11:08:27AM +0200, David Gow wrote: > > On Thu, Jun 30, 2022 at 9:29 PM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > > Stack trace collection code might trigger KASAN splats when walking > > > stack frames, but this can be resolved by using unchecked accesses. > > > The main reason to disable instrumentation here is for performance > > > reasons, see the upcoming patch for arm64 [1] for some details. > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 > > > > Ah -- that does it! Using READ_ONCE_NOCHECK() in dump_trace() gets rid > > of the nasty recursive KASAN failures we were getting in the tests. > > > > I'll send out v5 with those files instrumented again. > > Hmm, do we really want that? In the patch Andrey linked to above he > removed the READ_ONCE_NOCHECK() and added the KASAN_SANITIZE on the > corresponding files for arm64, just like it's already the case in this > patch for UML. Personally, I'm okay with the performance overhead so far: in my tests with a collection of ~350 KUnit tests, the total difference in runtime was about ~.2 seconds, and was within the margin of error caused by fluctuations in the compilation time. As an example, without the stacktrace code instrumented: [17:36:50] Testing complete. Passed: 364, Failed: 0, Crashed: 0, Skipped: 47, Errors: 0 [17:36:50] Elapsed time: 15.114s total, 0.003s configuring, 8.518s building, 6.433s running versus with it instrumented: [17:35:40] Testing complete. Passed: 364, Failed: 0, Crashed: 0, Skipped: 47, Errors: 0 [17:35:40] Elapsed time: 15.497s total, 0.003s configuring, 8.691s building, 6.640s running That being said, I'm okay with disabling it again and adding a comment if it's slow enough in some other usecase to cause problems (or even just be annoying). That could either be done in a v6 of this patchset, or a follow-up patch, depending on what people would prefer. But I'd not have a problem with leaving it instrumented for now. -- David [-- Attachment #2: S/MIME Cryptographic Signature --] [-- Type: application/pkcs7-signature, Size: 4003 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-07-01 9:43 ` David Gow @ 2022-07-01 10:04 ` Vincent Whitchurch 2022-07-01 10:34 ` Dmitry Vyukov 0 siblings, 1 reply; 10+ messages in thread From: Vincent Whitchurch @ 2022-07-01 10:04 UTC (permalink / raw) To: David Gow Cc: Andrey Konovalov, Dmitry Vyukov, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Fri, Jul 01, 2022 at 05:43:26PM +0800, David Gow wrote: > On Fri, Jul 1, 2022 at 5:16 PM Vincent Whitchurch > <vincent.whitchurch@axis.com> wrote: > > On Fri, Jul 01, 2022 at 11:08:27AM +0200, David Gow wrote: > > > On Thu, Jun 30, 2022 at 9:29 PM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > > > Stack trace collection code might trigger KASAN splats when walking > > > > stack frames, but this can be resolved by using unchecked accesses. > > > > The main reason to disable instrumentation here is for performance > > > > reasons, see the upcoming patch for arm64 [1] for some details. > > > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 > > > > > > Ah -- that does it! Using READ_ONCE_NOCHECK() in dump_trace() gets rid > > > of the nasty recursive KASAN failures we were getting in the tests. > > > > > > I'll send out v5 with those files instrumented again. > > > > Hmm, do we really want that? In the patch Andrey linked to above he > > removed the READ_ONCE_NOCHECK() and added the KASAN_SANITIZE on the > > corresponding files for arm64, just like it's already the case in this > > patch for UML. > > Personally, I'm okay with the performance overhead so far: in my tests > with a collection of ~350 KUnit tests, the total difference in runtime > was about ~.2 seconds, and was within the margin of error caused by > fluctuations in the compilation time. > > As an example, without the stacktrace code instrumented: > [17:36:50] Testing complete. Passed: 364, Failed: 0, Crashed: 0, > Skipped: 47, Errors: 0 > [17:36:50] Elapsed time: 15.114s total, 0.003s configuring, 8.518s > building, 6.433s running > > versus with it instrumented: > [17:35:40] Testing complete. Passed: 364, Failed: 0, Crashed: 0, > Skipped: 47, Errors: 0 > [17:35:40] Elapsed time: 15.497s total, 0.003s configuring, 8.691s > building, 6.640s running OK, good to know. > That being said, I'm okay with disabling it again and adding a comment > if it's slow enough in some other usecase to cause problems (or even > just be annoying). That could either be done in a v6 of this patchset, > or a follow-up patch, depending on what people would prefer. But I'd > not have a problem with leaving it instrumented for now. I don't have any strong opinion either way either, so you don't have to change it back on my account. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] UML: add support for KASAN under x86_64 2022-07-01 10:04 ` Vincent Whitchurch @ 2022-07-01 10:34 ` Dmitry Vyukov 0 siblings, 0 replies; 10+ messages in thread From: Dmitry Vyukov @ 2022-07-01 10:34 UTC (permalink / raw) To: Vincent Whitchurch Cc: David Gow, Andrey Konovalov, Johannes Berg, Patricia Alfonso, Jeff Dike, Richard Weinberger, anton.ivanov, Brendan Higgins, Andrew Morton, Andrey Ryabinin, kasan-dev, linux-um, LKML, Daniel Latypov, linux-mm, kunit-dev On Fri, 1 Jul 2022 at 12:04, Vincent Whitchurch <vincent.whitchurch@axis.com> wrote: > > <vincent.whitchurch@axis.com> wrote: > > > On Fri, Jul 01, 2022 at 11:08:27AM +0200, David Gow wrote: > > > > On Thu, Jun 30, 2022 at 9:29 PM Andrey Konovalov <andreyknvl@gmail.com> wrote: > > > > > Stack trace collection code might trigger KASAN splats when walking > > > > > stack frames, but this can be resolved by using unchecked accesses. > > > > > The main reason to disable instrumentation here is for performance > > > > > reasons, see the upcoming patch for arm64 [1] for some details. > > > > > > > > > > [1] https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/commit/?id=802b91118d11 > > > > > > > > Ah -- that does it! Using READ_ONCE_NOCHECK() in dump_trace() gets rid > > > > of the nasty recursive KASAN failures we were getting in the tests. > > > > > > > > I'll send out v5 with those files instrumented again. > > > > > > Hmm, do we really want that? In the patch Andrey linked to above he > > > removed the READ_ONCE_NOCHECK() and added the KASAN_SANITIZE on the > > > corresponding files for arm64, just like it's already the case in this > > > patch for UML. > > > > Personally, I'm okay with the performance overhead so far: in my tests > > with a collection of ~350 KUnit tests, the total difference in runtime > > was about ~.2 seconds, and was within the margin of error caused by > > fluctuations in the compilation time. > > > > As an example, without the stacktrace code instrumented: > > [17:36:50] Testing complete. Passed: 364, Failed: 0, Crashed: 0, > > Skipped: 47, Errors: 0 > > [17:36:50] Elapsed time: 15.114s total, 0.003s configuring, 8.518s > > building, 6.433s running > > > > versus with it instrumented: > > [17:35:40] Testing complete. Passed: 364, Failed: 0, Crashed: 0, > > Skipped: 47, Errors: 0 > > [17:35:40] Elapsed time: 15.497s total, 0.003s configuring, 8.691s > > building, 6.640s running > > OK, good to know. > > > That being said, I'm okay with disabling it again and adding a comment > > if it's slow enough in some other usecase to cause problems (or even > > just be annoying). That could either be done in a v6 of this patchset, > > or a follow-up patch, depending on what people would prefer. But I'd > > not have a problem with leaving it instrumented for now. > > I don't have any strong opinion either way either, so you don't have to > change it back on my account. Thanks. I would consider using READ_ONCE_NOCHECK() by default. And then switching to KASAN_SANITIZE:=n only if there is a real reason for that. Disabling instrumentation of any part of the kernel makes things faster, but at the same time we are losing checking coverage. For arm it was done for a very specific reason related to performance. While UML can be considered more test-oriented rather than production-oriented. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-07-01 10:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-30 8:08 [PATCH v4 1/2] mm: Add PAGE_ALIGN_DOWN macro David Gow 2022-06-30 8:08 ` [PATCH v4 2/2] UML: add support for KASAN under x86_64 David Gow 2022-06-30 9:41 ` Dmitry Vyukov 2022-06-30 12:54 ` Vincent Whitchurch 2022-06-30 13:28 ` Andrey Konovalov 2022-07-01 9:08 ` David Gow 2022-07-01 9:16 ` Vincent Whitchurch 2022-07-01 9:43 ` David Gow 2022-07-01 10:04 ` Vincent Whitchurch 2022-07-01 10:34 ` Dmitry Vyukov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).