Re: [RFC PATCH V1 0/8] KASAN ppc64 support
2015-08-17 12:50 GMT+03:00 Aneesh Kumar K.V : > Because of the above I concluded that we may not be able to do > inline instrumentation. Now if we are not doing inline instrumentation, > we can simplify kasan support by not creating a shadow mapping at all > for vmalloc and vmemmap region. Hence the idea of returning the address > of a zero page for anything other than kernel linear map region. > Yes, mapping zero page needed only for inline instrumentation. You simply don't need to check shadow for vmalloc/vmemmap. So, instead of redefining kasan_mem_to_shadow() I'd suggest to add one more arch hook. Something like: bool kasan_tracks_vaddr(unsigned long addr) { return REGION_ID(addr) == KERNEL_REGION_ID; } And in check_memory_region(): if (!(kasan_enabled() && kasan_tracks_vaddr(addr))) return; ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 4/8] kasan: Don't use kasan shadow pointer in generic functions
On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote: > We can't use generic functions like print_hex_dump to access kasan > shadow region. This require us to setup another kasan shadow region > for the address passed (kasan shadow address). Most architecture won't > be able to do that. Hence remove dumping kasan shadow region dump. If > we really want to do this we will have to have a kasan internal implemen > tation of print_hex_dump for which we will disable address sanitizer > operation. > I didn't understand that. Yes, you don't have shadow for shadow. But, for shadow addresses you return return (void *)kasan_zero_page in kasan_mem_to_shadow(), so we should be fine to access shadow in generic code. And with kasan_tracks_vaddr(), this should work too. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 7/8] powerpc/mm: kasan: Add kasan support for ppc64
On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote: > We use the region with region ID 0xe as the kasan shadow region. Since > we use hash page table, we can't have the early zero page based shadow > region support. Hence we disable kasan in the early code and runtime > enable this. We could imporve the condition using static keys. (but > that is for a later patch). We also can't support inline instrumentation > because our kernel mapping doesn't give us a large enough free window > to map the entire range. For VMALLOC and VMEMMAP region we just > return a zero page instead of having a translation bolted into the > htab. This simplifies handling VMALLOC and VMEMAP area. Kasan is not > tracking both the region as of now > > Signed-off-by: Aneesh Kumar K.V > --- > arch/powerpc/include/asm/kasan.h | 74 > > arch/powerpc/include/asm/pgtable-ppc64.h | 1 + > arch/powerpc/include/asm/ppc_asm.h | 10 + > arch/powerpc/include/asm/string.h| 13 ++ > arch/powerpc/kernel/Makefile | 1 + > arch/powerpc/kernel/prom_init_check.sh | 2 +- > arch/powerpc/kernel/setup_64.c | 3 ++ > arch/powerpc/lib/mem_64.S| 6 ++- > arch/powerpc/lib/memcpy_64.S | 3 +- > arch/powerpc/lib/ppc_ksyms.c | 10 + > arch/powerpc/mm/Makefile | 3 ++ > arch/powerpc/mm/kasan_init.c | 44 +++ > arch/powerpc/mm/slb_low.S| 4 ++ > arch/powerpc/platforms/Kconfig.cputype | 1 + > 14 files changed, 171 insertions(+), 4 deletions(-) > create mode 100644 arch/powerpc/include/asm/kasan.h > create mode 100644 arch/powerpc/mm/kasan_init.c > Did you disable stack instrumentation (in scripts/Makefile.kasa), or you version of gcc doesn't support it (e.g. like 4.9.x on x86) ? Because this can't work with stack instrumentation as you don't have shadow for stack in early code. But this should be doable, as I think. All you need is to setup shadow for init task's stack before executing any instrumented function. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 7/8] powerpc/mm: kasan: Add kasan support for ppc64
2015-08-17 15:13 GMT+03:00 Andrey Ryabinin : > > Did you disable stack instrumentation (in scripts/Makefile.kasa), > or you version of gcc doesn't support it (e.g. like 4.9.x on x86) ? > > Because this can't work with stack instrumentation as you don't have shadow > for stack in early code. > > But this should be doable, as I think. All you need is to setup shadow for > init task's > stack before executing any instrumented function. And you also need to define CONFIG_KASAN_SHADOW_OFFSET, so it will be passed to GCC via -fasan-shadow-offset= option. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 7/8] powerpc/mm: kasan: Add kasan support for ppc64
2015-08-18 8:36 GMT+03:00 Aneesh Kumar K.V : > Andrey Ryabinin writes: > >> 2015-08-17 15:13 GMT+03:00 Andrey Ryabinin : >>> >>> Did you disable stack instrumentation (in scripts/Makefile.kasa), >>> or you version of gcc doesn't support it (e.g. like 4.9.x on x86) ? >>> >>> Because this can't work with stack instrumentation as you don't have shadow >>> for stack in early code. >>> >>> But this should be doable, as I think. All you need is to setup shadow for >>> init task's >>> stack before executing any instrumented function. >> >> And you also need to define CONFIG_KASAN_SHADOW_OFFSET, so it will be >> passed to GCC >> via -fasan-shadow-offset= option. > > I am using KASAN minimal config. Hence this was not needed. Do we need > to pass that option for outline instrumentation ? If not it would be a > good idea to split that out and make it depend on KASAN_INLINE > We need to pass this for stack instrumentation too. > -aneesh > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 0/8] KASAN ppc64 support
2015-08-18 8:42 GMT+03:00 Aneesh Kumar K.V : > Andrey Ryabinin writes: > >> 2015-08-17 12:50 GMT+03:00 Aneesh Kumar K.V >> : >>> Because of the above I concluded that we may not be able to do >>> inline instrumentation. Now if we are not doing inline instrumentation, >>> we can simplify kasan support by not creating a shadow mapping at all >>> for vmalloc and vmemmap region. Hence the idea of returning the address >>> of a zero page for anything other than kernel linear map region. >>> >> >> Yes, mapping zero page needed only for inline instrumentation. >> You simply don't need to check shadow for vmalloc/vmemmap. >> >> So, instead of redefining kasan_mem_to_shadow() I'd suggest to >> add one more arch hook. Something like: >> >> bool kasan_tracks_vaddr(unsigned long addr) >> { >> return REGION_ID(addr) == KERNEL_REGION_ID; >> } >> >> And in check_memory_region(): >>if (!(kasan_enabled() && kasan_tracks_vaddr(addr))) >>return; > > > But that is introducting conditionals in core code for no real benefit. > This also will break when we eventually end up tracking vmalloc ? Ok, that's a very good reason to not do this. I see one potential problem in the way you use kasan_zero_page, though. memset/memcpy of large portions of memory ( > 8 * PAGE_SIZE) will end up in overflowing kasan_zero_page when we check shadow in memory_is_poisoned_n() > In that case our mem_to_shadow will esentially be a switch > statement returning different offsets for kernel region and vmalloc > region. As far as core kernel code is considered, it just need to > ask arch to get the shadow address for a memory and instead of adding > conditionals in core, my suggestion is, we handle this in an arch function. > > -aneesh > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 4/8] kasan: Don't use kasan shadow pointer in generic functions
2015-08-18 8:29 GMT+03:00 Aneesh Kumar K.V : > Andrey Ryabinin writes: > >> On 08/17/2015 09:36 AM, Aneesh Kumar K.V wrote: >>> We can't use generic functions like print_hex_dump to access kasan >>> shadow region. This require us to setup another kasan shadow region >>> for the address passed (kasan shadow address). Most architecture won't >>> be able to do that. Hence remove dumping kasan shadow region dump. If >>> we really want to do this we will have to have a kasan internal implemen >>> tation of print_hex_dump for which we will disable address sanitizer >>> operation. >>> >> >> I didn't understand that. >> Yes, you don't have shadow for shadow. But, for shadow addresses you >> return return (void *)kasan_zero_page in kasan_mem_to_shadow(), so we >> should be fine to access shadow in generic code. >> > > But in general IMHO it is not correct to pass shadow address to generic > functions, because that requires arch to setup shadow for the shadow. Yes, we have this shadow for shadow in x86_64/arm64. > With one of the initial implementation of ppc64 support, I had page > table entries setup for vmalloc and vmemmap shadow and that is when I > hit the issue. We cannot expect arch to setup shadow regions like what is > expected here. If we really need to print the shadow memory content, we > could possibly make a copy of print_hex_dump in kasan_init.c . Let me > know whether you think printing shadow area content is needed. > It was quite useful sometimes, so I think we should keep it. But I agree with you, that it would be better to avoid accesses to shadow memory in generic code. Another way to deal with this would be to copy shadow content in buffer, and then print_hex_dump() it. > -aneesh > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH V1 0/8] KASAN ppc64 support
2015-08-18 12:21 GMT+03:00 Aneesh Kumar K.V : > Andrey Ryabinin writes: > >> 2015-08-18 8:42 GMT+03:00 Aneesh Kumar K.V : >>> Andrey Ryabinin writes: >>> >>> >>> But that is introducting conditionals in core code for no real benefit. >>> This also will break when we eventually end up tracking vmalloc ? >> >> Ok, that's a very good reason to not do this. >> >> I see one potential problem in the way you use kasan_zero_page, though. >> memset/memcpy of large portions of memory ( > 8 * PAGE_SIZE) will end up >> in overflowing kasan_zero_page when we check shadow in memory_is_poisoned_n() >> > > Any suggestion on how to fix that ? I guess we definitely don't want to Wait, I was wrong, we should be fine. In memory_is_poisoned_n(): ret = memory_is_zero(kasan_mem_to_shadow((void *)addr), kasan_mem_to_shadow((void *)addr + size - 1) + 1); So this will be: memory_is_zero(kasan_zero_page, (char *)kasan_zero_page + 1); Which means that we will access only 1 byte of kasan_zero_page. > check for addr and size in memset/memcpy. The other option is to > do zero page mapping as is done for other architectures. That is we map > via page table a zero page. But we still have the issue of memory we > need to map the entire vmalloc range (page table memory). I was hoping to > avoid all those complexities. > > > -aneesh > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 00/10] KASan ppc64 support
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > Hi, > > This patchset implements kernel address sanitizer for ppc64. > Since ppc64 virtual address range is divided into different regions, > we can't have one contigous area for the kasan shadow range. Hence > we don't support the INLINE kasan instrumentation. With Outline > instrumentation, we override the shadow_to_mem and mem_to_shadow > callbacks, so that we map only the kernel linear range (ie, > region with ID 0xc). For region with ID 0xd and 0xf (vmalloc > and vmemmap ) we return the address of the zero page. This > works because kasan doesn't track both vmemmap and vmalloc address. > > Known issues: > * Kasan is not yet enabled for arch/powerpc/kvm > * kexec hang > * outline stack and global support > Is there any problem with globals or you just didn't try it yet? I think it should just work. You need only to add --param asan-globals=0 to KBUILD_CFLAGS_MODULE to disable it for modules. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 02/10] kasan: MODULE_VADDR is not available on all archs
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > Conditionalize the check using #ifdef > > Signed-off-by: Aneesh Kumar K.V > --- > mm/kasan/report.c | 11 --- > 1 file changed, 8 insertions(+), 3 deletions(-) > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c > index e07c94fbd0ac..71ce7548d914 100644 > --- a/mm/kasan/report.c > +++ b/mm/kasan/report.c > @@ -85,9 +85,14 @@ static void print_error_description(struct > kasan_access_info *info) > > static inline bool kernel_or_module_addr(const void *addr) > { > - return (addr >= (void *)_stext && addr < (void *)_end) > - || (addr >= (void *)MODULES_VADDR > - && addr < (void *)MODULES_END); > + if (addr >= (void *)_stext && addr < (void *)_end) > + return true; > +#if defined(CONFIG_MODULES) && defined(MODULES_VADDR) > + if (addr >= (void *)MODULES_VADDR > + && addr < (void *)MODULES_END) > + return true; > +#endif I don't think that this is correct change. On ppc64 modules are in VMALLOC, so you should check for this. Yes, we don't handle VMALLOC now, but we will at some point. So I think we should use is_module_address() here. It will be slower, but we don't care about performance in error reporting route. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 03/10] kasan: Rename kasan_enabled to kasan_report_enabled
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > The function only disable/enable reporting. In the later patch > we will be adding a kasan early enable/disable. Rename kasan_enabled > to properly reflect its function. > > Signed-off-by: Aneesh Kumar K.V Reviewed-by: Andrey Ryabinin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 04/10] kasan: Don't use kasan shadow pointer in generic functions
2015-08-26 11:54 GMT+03:00 Aneesh Kumar K.V : > > Missed to cherry-pick the updated version of this patch, before sending > the series out. > > commit aeb324e09d95c189eda4ce03790da94b535d1dfc > Author: Aneesh Kumar K.V > Date: Fri Aug 14 12:28:58 2015 +0530 > > kasan: Don't use kasan shadow pointer in generic functions > > We can't use generic functions like print_hex_dump to access kasan > shadow region. This require us to setup another kasan shadow region > for the address passed (kasan shadow address). Most architecture won't > be able to do that. Hence make a copy of the shadow region row and > pass that to generic functions. > > Signed-off-by: Aneesh Kumar K.V > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c > index d19d01823a68..60fdb0413f3b 100644 > --- a/mm/kasan/report.c > +++ b/mm/kasan/report.c > @@ -166,14 +166,20 @@ static void print_shadow_for_address(const void *addr) > for (i = -SHADOW_ROWS_AROUND_ADDR; i <= SHADOW_ROWS_AROUND_ADDR; i++) > { > const void *kaddr = kasan_shadow_to_mem(shadow_row); > char buffer[4 + (BITS_PER_LONG/8)*2]; > + char shadow_buf[SHADOW_BYTES_PER_ROW]; > > snprintf(buffer, sizeof(buffer), > (i == 0) ? ">%p: " : " %p: ", kaddr); > - > + /* > +* We should not pass a shadow pointer to generic > +* function, because generic functions may try to > +* kasan mapping for the passed address. may try to *access* kasan mapping? > +*/ > + memcpy(shadow_buf, shadow_row, SHADOW_BYTES_PER_ROW); > kasan_disable_current(); > print_hex_dump(KERN_ERR, buffer, > DUMP_PREFIX_NONE, SHADOW_BYTES_PER_ROW, 1, > - shadow_row, SHADOW_BYTES_PER_ROW, 0); > + shadow_buf, SHADOW_BYTES_PER_ROW, 0); > kasan_enable_current(); > > if (row_is_guilty(shadow_row, shadow)) > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 05/10] kasan: Enable arch to hook into kasan callbacks.
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > We add enable/disable callbacks in this patch which architecture > can implemement. We will use this in the later patches for architecture > like ppc64, that cannot have early zero page kasan shadow region for the > entire virtual address space. Such architectures also cannot use > inline kasan support. > > Signed-off-by: Aneesh Kumar K.V > --- > mm/kasan/kasan.c | 9 + > mm/kasan/kasan.h | 15 +++ > 2 files changed, 24 insertions(+) > > diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c > index 7b28e9cdf1c7..e4d33afd0eaf 100644 > --- a/mm/kasan/kasan.c > +++ b/mm/kasan/kasan.c > @@ -43,6 +43,9 @@ static void kasan_poison_shadow(const void *address, size_t > size, u8 value) > { > void *shadow_start, *shadow_end; > > + if (!kasan_enabled()) > + return; > + By the time this function called we already should have shadow, so it should be safe to remove this check. > shadow_start = kasan_mem_to_shadow(address); > shadow_end = kasan_mem_to_shadow(address + size); > > @@ -51,6 +54,9 @@ static void kasan_poison_shadow(const void *address, size_t > size, u8 value) > > void kasan_unpoison_shadow(const void *address, size_t size) > { > + if (!kasan_enabled()) > + return; > + Ditto. > kasan_poison_shadow(address, size, 0); > > if (size & KASAN_SHADOW_MASK) { > @@ -238,6 +244,9 @@ static __always_inline void check_memory_region(unsigned > long addr, > { > struct kasan_access_info info; > > + if (!kasan_enabled()) > + return; > + > if (unlikely(size == 0)) > return; > > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index a6b46cc94907..deb547d5a916 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -68,6 +68,21 @@ static inline bool kasan_report_enabled(void) > return !current->kasan_depth; > } > > +#ifndef kasan_enabled > +/* > + * Some archs may want to disable kasan callbacks. > + */ > +static inline bool kasan_enabled(void) > +{ > + return true; > +} > +#define kasan_enabled kasan_enabled Why we need this define? > +#else > +#ifdef CONFIG_KASAN_INLINE > +#error "Kasan inline support cannot work with KASAN arch hooks" > +#endif > +#endif > + > void kasan_report(unsigned long addr, size_t size, > bool is_write, unsigned long ip); > > -- > 2.5.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 06/10] kasan: Allow arch to overrride kasan shadow offsets
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > Some archs may want to provide kasan shadow memory as a constant > offset from the address. Such arch even though cannot use inline kasan > support, they can work with outofline kasan support. > > Signed-off-by: Aneesh Kumar K.V > --- > include/linux/kasan.h | 3 +++ > mm/kasan/kasan.h | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index 5486d777b706..e458ca64cdaf 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -15,11 +15,14 @@ struct vm_struct; > #include > #include > > +#ifndef kasan_mem_to_shadow > static inline void *kasan_mem_to_shadow(const void *addr) > { > return (void *)((unsigned long)addr >> KASAN_SHADOW_SCALE_SHIFT) > + KASAN_SHADOW_OFFSET; > } > +#define kasan_mem_to_shadow kasan_mem_to_shadow Again, why we need this define here? I think it should be safe to remove it. > +#endif > > /* Enable reporting bugs after kasan_disable_current() */ > static inline void kasan_enable_current(void) > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index deb547d5a916..c0686f2b1224 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -57,11 +57,14 @@ struct kasan_global { > void kasan_report_error(struct kasan_access_info *info); > void kasan_report_user_access(struct kasan_access_info *info); > > +#ifndef kasan_shadow_to_mem > static inline const void *kasan_shadow_to_mem(const void *shadow_addr) > { > return (void *)(((unsigned long)shadow_addr - KASAN_SHADOW_OFFSET) > << KASAN_SHADOW_SCALE_SHIFT); > } > +#define kasan_shadow_to_mem kasan_shadow_to_mem ditto > +#endif > > static inline bool kasan_report_enabled(void) > { > -- > 2.5.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 07/10] kasan: Make INLINE KASan support arch selectable
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > Some of the archs, may find it difficult to support inline KASan > mode. Add HAVE_ARCH_KASAN_INLINE so that we can disable inline > support at config time. > > Signed-off-by: Aneesh Kumar K.V > --- > arch/x86/Kconfig | 1 + > lib/Kconfig.kasan | 2 ++ > scripts/Makefile.kasan | 28 ++-- > 3 files changed, 17 insertions(+), 14 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index b3a1a5d77d92..4416f80580fb 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -78,6 +78,7 @@ config X86 > select HAVE_ARCH_HUGE_VMAP if X86_64 || X86_PAE > select HAVE_ARCH_JUMP_LABEL > select HAVE_ARCH_KASAN if X86_64 && SPARSEMEM_VMEMMAP > + select HAVE_ARCH_KASAN_INLINE if X86_64 && > SPARSEMEM_VMEMMAP This will not work because config HAVE_ARCH_KASAN_INLINE is not defined. Instead of you can just add following in this file: config HAVE_ARCH_KASAN_INLINE def_bool y depends on KASAN > select HAVE_ARCH_KGDB > select HAVE_ARCH_KMEMCHECK > select HAVE_ARCH_SECCOMP_FILTER > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index 39f24d6721e5..e9d1bb1175b8 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -32,6 +32,7 @@ config KASAN_OUTLINE > however it doesn't bloat size of kernel's .text section so > much as inline does. > > +if HAVE_ARCH_KASAN_INLINE > config KASAN_INLINE > bool "Inline instrumentation" depends on HAVE_ARCH_KASAN_INLINE > help > @@ -40,6 +41,7 @@ config KASAN_INLINE > it gives about x2 boost over outline instrumentation), but > make kernel's .text size much bigger. > This requires a gcc version of 5.0 or later. > +endif > > endchoice > > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan > index 3f874d24234f..c1c06e9e107a 100644 > --- a/scripts/Makefile.kasan > +++ b/scripts/Makefile.kasan > @@ -1,29 +1,29 @@ > ifdef CONFIG_KASAN > -ifdef CONFIG_KASAN_INLINE > - call_threshold := 1 > -else > - call_threshold := 0 > -endif > - > -CFLAGS_KASAN_MINIMAL := -fsanitize=kernel-address > > CFLAGS_KASAN := $(call cc-option, -fsanitize=kernel-address \ > - -fasan-shadow-offset=$(CONFIG_KASAN_SHADOW_OFFSET) \ > - --param asan-stack=1 --param asan-globals=1 \ > - --param > asan-instrumentation-with-call-threshold=$(call_threshold)) > - > -ifeq ($(call cc-option, $(CFLAGS_KASAN_MINIMAL) -Werror),) > + --param asan-instrumentation-with-call-threshold=0) > +ifeq ($(CFLAGS_KASAN),) > ifneq ($(CONFIG_COMPILE_TEST),y) > $(warning Cannot use CONFIG_KASAN: \ > -fsanitize=kernel-address is not supported by compiler) > endif > else > -ifeq ($(CFLAGS_KASAN),) > + > + ifdef CONFIG_KASAN_INLINE > + CFLAGS_KASAN_INLINE := $(call cc-option, -fsanitize=kernel-address \ > +-fasan-shadow-offset=$(CONFIG_KASAN_SHADOW_OFFSET) \ > +--param asan-stack=1 --param asan-globals=1 \ > +--param > asan-instrumentation-with-call-threshold=1) > + > +ifeq ($(CFLAGS_KASAN_INLINE),) > ifneq ($(CONFIG_COMPILE_TEST),y) > $(warning CONFIG_KASAN: compiler does not support all options.\ > Trying minimal configuration) > endif > -CFLAGS_KASAN := $(CFLAGS_KASAN_MINIMAL) > +else > +CFLAGS_KASAN := $(CFLAGS_KASAN_INLINE) > endif > +endif > + This removes stack and globals for CONFIG_KASAN_OUTLINE=y. Why? Those are completely separate features. So this patch shouldn't touch this Makefile at all. Depends on HAVE_ARCH_KASAN_INLINE in CONFIG_KASAN_INLINE should be enough. But you need to disable 'asan-stack' and 'asan-globals' for pcc64. I'd suggest to introduce CFLAGS_ARCH_KASAN. Define it in ppc64 Makefile: CFLAGS_ARCH_KASAN := --param asan-globals=0 --param asan-stack=0 and add these flags to CFLAGS_KASAN_MINIMAL and CFLAGS_KASAN in Makefile.kasan. > endif > endif > -- > 2.5.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 09/10] kasan: Prevent deadlock in kasan reporting
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > We we end up calling kasan_report in real mode, our shadow mapping > for even spinlock variable will show poisoned. Generally I agree with this patch. We should disable reports when we print report as early as possible to prevent recursion in case of bug in spinlock or printk etc. But I don't understand what is the problem that you observing. How we ended up with shadow poisoned for a valid spinlock struct? And since shadow poisoned for some valid memory we should get enormous amount of false positive reports. > This will result > in us calling kasan_report_error with lock_report spin lock held. > To prevent this disable kasan reporting when we are priting > error w.r.t kasan. > > Signed-off-by: Aneesh Kumar K.V > --- > mm/kasan/report.c | 10 ++ > 1 file changed, 10 insertions(+) > > diff --git a/mm/kasan/report.c b/mm/kasan/report.c > index 79fbc5d14bd2..82b41eb83e43 100644 > --- a/mm/kasan/report.c > +++ b/mm/kasan/report.c > @@ -185,6 +185,10 @@ void kasan_report_error(struct kasan_access_info *info) > { > unsigned long flags; > > + /* > +* Make sure we don't end up in loop. > +*/ > + kasan_disable_current(); > spin_lock_irqsave(&report_lock, flags); > pr_err("=" > "=\n"); > @@ -194,12 +198,17 @@ void kasan_report_error(struct kasan_access_info *info) > pr_err("=" > "=\n"); > spin_unlock_irqrestore(&report_lock, flags); > + kasan_enable_current(); > } > > void kasan_report_user_access(struct kasan_access_info *info) > { > unsigned long flags; > > + /* > +* Make sure we don't end up in loop. > +*/ > + kasan_disable_current(); > spin_lock_irqsave(&report_lock, flags); > pr_err("=" > "=\n"); > @@ -212,6 +221,7 @@ void kasan_report_user_access(struct kasan_access_info > *info) > pr_err("=" > "=\n"); > spin_unlock_irqrestore(&report_lock, flags); > + kasan_enable_current(); > } > > void kasan_report(unsigned long addr, size_t size, > -- > 2.5.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 10/10] powerpc/mm: kasan: Add kasan support for ppc64
2015-08-26 11:26 GMT+03:00 Aneesh Kumar K.V : > + k_start = (unsigned long)kasan_mem_to_shadow(start); > + k_end = (unsigned long)kasan_mem_to_shadow(end); > + for (; k_start < k_end; k_start += page_size) { > + p = vmemmap_alloc_block(page_size, node); > + if (!p) { > + pr_info("Disabled Kasan, for lack of free > mem\n"); > + /* Free the stuff or panic ? */ vmemmap_alloc_block() panics on allocation failure, so you don't need this if block. You could replace this with memblock_virt_alloc_try_nid_nopanic(), but note that if/when we will have working asan-stack=1 there will be no way for fallback. > + return; > + } > + htab_bolt_mapping(k_start, k_start + page_size, > + __pa(p), pgprot_val(PAGE_KERNEL), > + mmu_vmemmap_psize, > mmu_kernel_ssize); > + } > + } > + /* > +* At this point kasan is fully initialized. Enable error messages > +*/ > + init_task.kasan_depth = 0; > + __kasan_enabled = true; > + pr_info("Kernel address sanitizer initialized\n"); > +} > diff --git a/arch/powerpc/mm/slb_low.S b/arch/powerpc/mm/slb_low.S > index 736d18b3cefd..154bd8a0b437 100644 > --- a/arch/powerpc/mm/slb_low.S > +++ b/arch/powerpc/mm/slb_low.S > @@ -80,11 +80,15 @@ END_MMU_FTR_SECTION_IFCLR(MMU_FTR_1T_SEGMENT) > /* Check virtual memmap region. To be patches at kernel boot */ > cmpldi cr0,r9,0xf > bne 1f > +2: > .globl slb_miss_kernel_load_vmemmap > slb_miss_kernel_load_vmemmap: > li r11,0 > b 6f > 1: > + /* Kasan region same as vmemmap mapping */ > + cmpldi cr0,r9,0xe > + beq 2b > #endif /* CONFIG_SPARSEMEM_VMEMMAP */ > > /* vmalloc mapping gets the encoding from the PACA as the mapping > diff --git a/arch/powerpc/platforms/Kconfig.cputype > b/arch/powerpc/platforms/Kconfig.cputype > index c140e94c7c72..7a7c9d54f80e 100644 > --- a/arch/powerpc/platforms/Kconfig.cputype > +++ b/arch/powerpc/platforms/Kconfig.cputype > @@ -75,6 +75,7 @@ config PPC_BOOK3S_64 > select HAVE_ARCH_TRANSPARENT_HUGEPAGE if PPC_64K_PAGES > select ARCH_SUPPORTS_NUMA_BALANCING > select IRQ_WORK > + select HAVE_ARCH_KASAN if SPARSEMEM_VMEMMAP > > config PPC_BOOK3E_64 > bool "Embedded processors" > -- > 2.5.0 > ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: BUG: KASAN: stack-out-of-bounds
On 2/27/19 11:25 AM, Christophe Leroy wrote: > With version v8 of the series implementing KASAN on 32 bits powerpc > (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm > now able to activate KASAN on a mac99 is QEMU. > > Then I get the following reports at startup. Which of the two reports I get > seems to depend on the option used to build the kernel, but for a given > kernel I always get the same report. > > Is that a real bug, in which case how could I spot it ? Or is it something > wrong in my implementation of KASAN ? > > I checked that after kasan_init(), the entire shadow memory is full of 0 only. > > I also made a try with the strong STACK_PROTECTOR compiled in, but no > difference and nothing detected by the stack protector. > > == > BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74 > Read of size 1 at addr c0ecdd40 by task swapper/0 > > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133 > Call Trace: > [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable) > [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180 > [c0e9dd10] [c089579c] memchr+0x24/0x74 > [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574 > [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8 > [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4 > --- interrupt: c0e9df00 at 0x400f330 > LR = init_stack+0x1f00/0x2000 > [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable) > [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108 > [c0e9df50] [c0c16434] start_kernel+0x310/0x488 > [c0e9dff0] [3484] 0x3484 > > The buggy address belongs to the variable: > __log_buf+0xec0/0x4020 > The buggy address belongs to the page: > page:c6eac9a0 count:1 mapcount:0 mapping: index:0x0 > flags: 0x1000(reserved) > raw: 1000 c6eac9a4 c6eac9a4 0001 > page dumped because: kasan: bad access detected > > Memory state around the buggy address: > c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 > ^ > c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 > c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 > == > This one doesn't look good. Notice that it says stack-out-of-bounds, but at the same time there is "The buggy address belongs to the variable: __log_buf+0xec0/0x4020" which is printed by following code: if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) { pr_err("The buggy address belongs to the variable:\n"); pr_err(" %pS\n", addr); } So the stack unrelated address got stack-related poisoning. This could be a stack overflow, did you increase THREAD_SHIFT? KASAN with stack instrumentation significantly increases stack usage.
Re: BUG: KASAN: stack-out-of-bounds
On 2/27/19 4:11 PM, Christophe Leroy wrote: > > > Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit : >> >> >> On 2/27/19 11:25 AM, Christophe Leroy wrote: >>> With version v8 of the series implementing KASAN on 32 bits powerpc >>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), I'm >>> now able to activate KASAN on a mac99 is QEMU. >>> >>> Then I get the following reports at startup. Which of the two reports I get >>> seems to depend on the option used to build the kernel, but for a given >>> kernel I always get the same report. >>> >>> Is that a real bug, in which case how could I spot it ? Or is it something >>> wrong in my implementation of KASAN ? >>> >>> I checked that after kasan_init(), the entire shadow memory is full of 0 >>> only. >>> >>> I also made a try with the strong STACK_PROTECTOR compiled in, but no >>> difference and nothing detected by the stack protector. >>> >>> == >>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74 >>> Read of size 1 at addr c0ecdd40 by task swapper/0 >>> >>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133 >>> Call Trace: >>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable) >>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180 >>> [c0e9dd10] [c089579c] memchr+0x24/0x74 >>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574 >>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8 >>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4 >>> --- interrupt: c0e9df00 at 0x400f330 >>> LR = init_stack+0x1f00/0x2000 >>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable) >>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108 >>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488 >>> [c0e9dff0] [3484] 0x3484 >>> >>> The buggy address belongs to the variable: >>> __log_buf+0xec0/0x4020 >>> The buggy address belongs to the page: >>> page:c6eac9a0 count:1 mapcount:0 mapping: index:0x0 >>> flags: 0x1000(reserved) >>> raw: 1000 c6eac9a4 c6eac9a4 0001 >>> page dumped because: kasan: bad access detected >>> >>> Memory state around the buggy address: >>> c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>> c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 >>> ^ >>> c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 >>> c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>> == >>> >> >> This one doesn't look good. Notice that it says stack-out-of-bounds, but at >> the same time there is >> "The buggy address belongs to the variable: __log_buf+0xec0/0x4020" >> which is printed by following code: >> if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) { >> pr_err("The buggy address belongs to the variable:\n"); >> pr_err(" %pS\n", addr); >> } >> >> So the stack unrelated address got stack-related poisoning. This could be a >> stack overflow, did you increase THREAD_SHIFT? >> KASAN with stack instrumentation significantly increases stack usage. >> > > I get the above with THREAD_SHIFT set to 13 (default value). > If increasing it to 14, I get the following instead. That means that in that > case the problem arises a lot earlier in the boot process (but still after > the final kasan shadow setup). > We usually use 15 (with 4k pages), but I think 14 should be enough for the clean boot. > == > BUG: KASAN: stack-out-of-bounds in pmac_nvram_init+0x1f8/0x5d0 > Read of size 1 at addr f6f37de0 by task swapper/0 > > CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1143 > Call Trace: > [c0e9fd60] [c01c43c0] print_address_description+0x164/0x2bc (unreliable) > [c0e9fd90] [c01c46a4] kasan_report+0xfc/0x180 > [c0e9fdd0] [c0c226d4] pmac_nvram_init+0x1f8/0x5d0 > [c0e9fef0] [c0c1f73c] pmac_setup_arch+0x298/0x314 > [c0e9ff20] [c0c1ac40] setup_arch+0x250/0x268 > [c0e9ff50] [c0c151dc] start_kernel+0xb8/0x488 > [c0e9fff0] [3484] 0x3484 > > > Memory state around the buggy address: > f6f37c80: 00 00 00 00 00 00 00 00 00 0
Re: BUG: KASAN: stack-out-of-bounds
On 2/28/19 12:27 PM, Dmitry Vyukov wrote: > On Thu, Feb 28, 2019 at 10:22 AM Andrey Ryabinin > wrote: >> >> >> >> On 2/27/19 4:11 PM, Christophe Leroy wrote: >>> >>> >>> Le 27/02/2019 à 10:19, Andrey Ryabinin a écrit : >>>> >>>> >>>> On 2/27/19 11:25 AM, Christophe Leroy wrote: >>>>> With version v8 of the series implementing KASAN on 32 bits powerpc >>>>> (https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=94309), >>>>> I'm now able to activate KASAN on a mac99 is QEMU. >>>>> >>>>> Then I get the following reports at startup. Which of the two reports I >>>>> get seems to depend on the option used to build the kernel, but for a >>>>> given kernel I always get the same report. >>>>> >>>>> Is that a real bug, in which case how could I spot it ? Or is it >>>>> something wrong in my implementation of KASAN ? >>>>> >>>>> I checked that after kasan_init(), the entire shadow memory is full of 0 >>>>> only. >>>>> >>>>> I also made a try with the strong STACK_PROTECTOR compiled in, but no >>>>> difference and nothing detected by the stack protector. >>>>> >>>>> == >>>>> BUG: KASAN: stack-out-of-bounds in memchr+0x24/0x74 >>>>> Read of size 1 at addr c0ecdd40 by task swapper/0 >>>>> >>>>> CPU: 0 PID: 0 Comm: swapper Not tainted 5.0.0-rc7+ #1133 >>>>> Call Trace: >>>>> [c0e9dca0] [c01c42a0] print_address_description+0x64/0x2bc (unreliable) >>>>> [c0e9dcd0] [c01c4684] kasan_report+0xfc/0x180 >>>>> [c0e9dd10] [c089579c] memchr+0x24/0x74 >>>>> [c0e9dd30] [c00a9e38] msg_print_text+0x124/0x574 >>>>> [c0e9dde0] [c00ab710] console_unlock+0x114/0x4f8 >>>>> [c0e9de40] [c00adc60] vprintk_emit+0x188/0x1c4 >>>>> --- interrupt: c0e9df00 at 0x400f330 >>>>> LR = init_stack+0x1f00/0x2000 >>>>> [c0e9de80] [c00ae3c4] printk+0xa8/0xcc (unreliable) >>>>> [c0e9df20] [c0c28e44] early_irq_init+0x38/0x108 >>>>> [c0e9df50] [c0c16434] start_kernel+0x310/0x488 >>>>> [c0e9dff0] [3484] 0x3484 >>>>> >>>>> The buggy address belongs to the variable: >>>>> __log_buf+0xec0/0x4020 >>>>> The buggy address belongs to the page: >>>>> page:c6eac9a0 count:1 mapcount:0 mapping: index:0x0 >>>>> flags: 0x1000(reserved) >>>>> raw: 1000 c6eac9a4 c6eac9a4 >>>>> 0001 >>>>> page dumped because: kasan: bad access detected >>>>> >>>>> Memory state around the buggy address: >>>>> c0ecdc00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>>>> c0ecdc80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>>>>> c0ecdd00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 00 >>>>> ^ >>>>> c0ecdd80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 >>>>> c0ecde00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 >>>>> == >>>>> >>>> >>>> This one doesn't look good. Notice that it says stack-out-of-bounds, but >>>> at the same time there is >>>> "The buggy address belongs to the variable: __log_buf+0xec0/0x4020" >>>> which is printed by following code: >>>> if (kernel_or_module_addr(addr) && !init_task_stack_addr(addr)) { >>>> pr_err("The buggy address belongs to the variable:\n"); >>>> pr_err(" %pS\n", addr); >>>> } >>>> >>>> So the stack unrelated address got stack-related poisoning. This could be >>>> a stack overflow, did you increase THREAD_SHIFT? >>>> KASAN with stack instrumentation significantly increases stack usage. >>>> >>> >>> I get the above with THREAD_SHIFT set to 13 (default value). >>> If increasing it to 14, I get the following instead. That means that in >>> that case the problem arises a lot earlier in the boot process (but still >>> after the final kasan shadow set
Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions
On 4/2/19 12:43 PM, Christophe Leroy wrote: > Hi Dmitry, Andrey and others, > > Do you have any comments to this series ? > I don't see justification for adding all these non-instrumented functions. We need only some subset of these functions and only on powerpc so far. Arches that don't use str*() that early simply doesn't need not-instrumented __str*() variant. Also I don't think that auto-replace str* to __str* for all not instrumented files is a good idea, as this will reduce KASAN coverage. E.g. we don't instrument slub.c but there is no reason to use non-instrumented __str*() functions there. And finally, this series make bug reporting slightly worse. E.g. let's look at strcpy(): +char *strcpy(char *dest, const char *src) +{ + size_t len = __strlen(src) + 1; + + check_memory_region((unsigned long)src, len, false, _RET_IP_); + check_memory_region((unsigned long)dest, len, true, _RET_IP_); + + return __strcpy(dest, src); +} If src is not-null terminated string we might not see proper out-of-bounds report from KASAN only a crash in __strlen(). Which might make harder to identify where 'src' comes from, where it was allocated and what's the size of allocated area. > I'd like to know if this approach is ok or if it is better to keep doing as > in https://patchwork.ozlabs.org/patch/1055788/ > I think the patch from link is a better solution to the problem.
Re: [PATCH v10 1/5] kasan: support backing vmalloc space with real shadow memory
On 10/29/19 7:20 AM, Daniel Axtens wrote: > Hook into vmalloc and vmap, and dynamically allocate real shadow > memory to back the mappings. > > Most mappings in vmalloc space are small, requiring less than a full > page of shadow space. Allocating a full shadow page per mapping would > therefore be wasteful. Furthermore, to ensure that different mappings > use different shadow pages, mappings would have to be aligned to > KASAN_SHADOW_SCALE_SIZE * PAGE_SIZE. > > Instead, share backing space across multiple mappings. Allocate a > backing page when a mapping in vmalloc space uses a particular page of > the shadow region. This page can be shared by other vmalloc mappings > later on. > > We hook in to the vmap infrastructure to lazily clean up unused shadow > memory. > > To avoid the difficulties around swapping mappings around, this code > expects that the part of the shadow region that covers the vmalloc > space will not be covered by the early shadow page, but will be left > unmapped. This will require changes in arch-specific code. > > This allows KASAN with VMAP_STACK, and may be helpful for architectures > that do not have a separate module space (e.g. powerpc64, which I am > currently working on). It also allows relaxing the module alignment > back to PAGE_SIZE. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202009 > Acked-by: Vasily Gorbik > Co-developed-by: Mark Rutland > Signed-off-by: Mark Rutland [shadow rework] > Signed-off-by: Daniel Axtens Small nit bellow, otherwise looks fine: Reviewed-by: Andrey Ryabinin > static __always_inline bool > @@ -1196,8 +1201,8 @@ static void free_vmap_area(struct vmap_area *va) >* Insert/Merge it back to the free tree/list. >*/ > spin_lock(&free_vmap_area_lock); > - merge_or_add_vmap_area(va, > - &free_vmap_area_root, &free_vmap_area_list); > + (void)merge_or_add_vmap_area(va, &free_vmap_area_root, > + &free_vmap_area_list); > spin_unlock(&free_vmap_area_lock); > } > .. > > @@ -3391,8 +3428,8 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned > long *offsets, >* and when pcpu_get_vm_areas() is success. >*/ > while (area--) { > - merge_or_add_vmap_area(vas[area], > - &free_vmap_area_root, &free_vmap_area_list); > + (void)merge_or_add_vmap_area(vas[area], &free_vmap_area_root, I don't think these (void) casts are necessary. > + &free_vmap_area_list); > vas[area] = NULL; > } > >
Re: [PATCH v10 2/5] kasan: add test for vmalloc
On 10/29/19 7:20 AM, Daniel Axtens wrote: > Test kasan vmalloc support by adding a new test to the module. > > Signed-off-by: Daniel Axtens > Reviewed-by: Andrey Ryabinin
Re: [PATCH v10 3/5] fork: support VMAP_STACK with KASAN_VMALLOC
On 10/29/19 7:20 AM, Daniel Axtens wrote: > Supporting VMAP_STACK with KASAN_VMALLOC is straightforward: > > - clear the shadow region of vmapped stacks when swapping them in > - tweak Kconfig to allow VMAP_STACK to be turned on with KASAN > > Reviewed-by: Dmitry Vyukov > Signed-off-by: Daniel Axtens > --- Reviewed-by: Andrey Ryabinin > > diff --git a/kernel/fork.c b/kernel/fork.c > index 954e875e72b1..a6e5249ad74b 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -94,6 +94,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -224,6 +225,9 @@ static unsigned long *alloc_thread_stack_node(struct > task_struct *tsk, int node) > if (!s) > continue; > > + /* Clear the KASAN shadow of the stack. */ > + kasan_unpoison_shadow(s->addr, THREAD_SIZE); > + Just sharing the thought. We could possibly add poisoning in free_thread_stack() to catch possible usage of freed cached stack. But it might be a bad idea because cached stacks supposed to be reused very quickly. So it might just add overhead without much gain. > /* Clear stale pointers from reused stack. */ > memset(s->addr, 0, THREAD_SIZE); > >
Re: [PATCH v10 4/5] x86/kasan: support KASAN_VMALLOC
On 10/29/19 7:20 AM, Daniel Axtens wrote: > In the case where KASAN directly allocates memory to back vmalloc > space, don't map the early shadow page over it. > > We prepopulate pgds/p4ds for the range that would otherwise be empty. > This is required to get it synced to hardware on boot, allowing the > lower levels of the page tables to be filled dynamically. > > Acked-by: Dmitry Vyukov > Signed-off-by: Daniel Axtens > > --- > +static void __init kasan_shallow_populate_pgds(void *start, void *end) > +{ > + unsigned long addr, next; > + pgd_t *pgd; > + void *p; > + int nid = early_pfn_to_nid((unsigned long)start); This doesn't make sense. start is not even a pfn. With linear mapping we try to identify nid to have the shadow on the same node as memory. But in this case we don't have memory or the corresponding shadow (yet), we only install pgd/p4d. I guess we could just use NUMA_NO_NODE. The rest looks ok, so with that fixed: Reviewed-by: Andrey Ryabinin
Re: [PATCH v10 5/5] kasan debug: track pages allocated for vmalloc shadow
On 10/29/19 7:20 AM, Daniel Axtens wrote: > Provide the current number of vmalloc shadow pages in > /sys/kernel/debug/kasan/vmalloc_shadow_pages. > I wouldn't merge this. I don't see use-case for this, besides testing this patch set. And I think that number should be possible to extract via page_owner mechanism.
Re: [PATCH v10 4/5] x86/kasan: support KASAN_VMALLOC
On 10/30/19 4:50 PM, Daniel Axtens wrote: > Andrey Ryabinin writes: > >> On 10/29/19 7:20 AM, Daniel Axtens wrote: >>> In the case where KASAN directly allocates memory to back vmalloc >>> space, don't map the early shadow page over it. >>> >>> We prepopulate pgds/p4ds for the range that would otherwise be empty. >>> This is required to get it synced to hardware on boot, allowing the >>> lower levels of the page tables to be filled dynamically. >>> >>> Acked-by: Dmitry Vyukov >>> Signed-off-by: Daniel Axtens >>> >>> --- >> >>> +static void __init kasan_shallow_populate_pgds(void *start, void *end) >>> +{ >>> + unsigned long addr, next; >>> + pgd_t *pgd; >>> + void *p; >>> + int nid = early_pfn_to_nid((unsigned long)start); >> >> This doesn't make sense. start is not even a pfn. With linear mapping >> we try to identify nid to have the shadow on the same node as memory. But >> in this case we don't have memory or the corresponding shadow (yet), >> we only install pgd/p4d. >> I guess we could just use NUMA_NO_NODE. > > Ah wow, that's quite the clanger on my part. > > There are a couple of other invocations of early_pfn_to_nid in that file > that use an address directly, but at least they reference actual memory. > I'll send a separate patch to fix those up. I see only one incorrect, in kasan_init(): early_pfn_to_nid(__pa(_stext)) It should be wrapped with PFN_DOWN(). Other usages in map_range() seems to be correct, range->start,end is pfns. > >> The rest looks ok, so with that fixed: >> >> Reviewed-by: Andrey Ryabinin > > Thanks heaps! I've fixed up the nit you identifed in the first patch, > and I agree that the last patch probably isn't needed. I'll respin the > series shortly. > Hold on a sec, just spotted another thing to fix. > @@ -352,9 +397,24 @@ void __init kasan_init(void) > shadow_cpu_entry_end = (void *)round_up( > (unsigned long)shadow_cpu_entry_end, PAGE_SIZE); > > + /* > + * If we're in full vmalloc mode, don't back vmalloc space with early > + * shadow pages. Instead, prepopulate pgds/p4ds so they are synced to > + * the global table and we can populate the lower levels on demand. > + */ > +#ifdef CONFIG_KASAN_VMALLOC > + kasan_shallow_populate_pgds( > + kasan_mem_to_shadow((void *)PAGE_OFFSET + MAXMEM), This should be VMALLOC_START, there is no point to allocate pgds for the hole between linear mapping and vmalloc, just waste of memory. It make sense to map early shadow for that hole, because if code dereferences address in that hole we will see the page fault on that address instead of fault on the shadow. So something like this might work: kasan_populate_early_shadow( kasan_mem_to_shadow((void *)PAGE_OFFSET + MAXMEM), kasan_mem_to_shadow((void *)VMALLOC_START)); if (IS_ENABLED(CONFIG_KASAN_VMALLOC) kasan_shallow_populate_pgds(kasan_mem_to_shadow(VMALLOC_START), kasan_mem_to_shadow((void *)VMALLOC_END)) else kasan_populate_early_shadow(kasan_mem_to_shadow(VMALLOC_START), kasan_mem_to_shadow((void *)VMALLOC_END)); kasan_populate_early_shadow( kasan_mem_to_shadow((void *)VMALLOC_END + 1), shadow_cpu_entry_begin);
Re: [PATCH v11 0/4] kasan: support backing vmalloc space with real shadow memory
On 10/31/19 12:39 PM, Daniel Axtens wrote: > Daniel Axtens (4): > kasan: support backing vmalloc space with real shadow memory > kasan: add test for vmalloc > fork: support VMAP_STACK with KASAN_VMALLOC > x86/kasan: support KASAN_VMALLOC > > Documentation/dev-tools/kasan.rst | 63 > arch/Kconfig | 9 +- > arch/x86/Kconfig | 1 + > arch/x86/mm/kasan_init_64.c | 61 > include/linux/kasan.h | 31 > include/linux/moduleloader.h | 2 +- > include/linux/vmalloc.h | 12 ++ > kernel/fork.c | 4 + > lib/Kconfig.kasan | 16 +++ > lib/test_kasan.c | 26 > mm/kasan/common.c | 231 ++ > mm/kasan/generic_report.c | 3 + > mm/kasan/kasan.h | 1 + > mm/vmalloc.c | 53 +-- > 14 files changed, 500 insertions(+), 13 deletions(-) > Andrew, could pick this up please?
Re: [PATCH v11 1/4] kasan: support backing vmalloc space with real shadow memory
On 11/18/19 6:29 AM, Daniel Axtens wrote: > Qian Cai writes: > >> On Thu, 2019-10-31 at 20:39 +1100, Daniel Axtens wrote: >>> /* >>> * In this function, newly allocated vm_struct has VM_UNINITIALIZED >>> * flag. It means that vm_struct is not fully initialized. >>> @@ -3377,6 +3411,9 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned >>> long *offsets, >>> >>> setup_vmalloc_vm_locked(vms[area], vas[area], VM_ALLOC, >>> pcpu_get_vm_areas); >>> + >>> + /* assume success here */ >>> + kasan_populate_vmalloc(sizes[area], vms[area]); >>> } >>> spin_unlock(&vmap_area_lock); >> >> Here it is all wrong. GFP_KERNEL with in_atomic(). > > I think this fix will work, I will do a v12 with it included. You can send just the fix. Andrew will fold it into the original patch before sending it to Linus. > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index a4b950a02d0b..bf030516258c 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -3417,11 +3417,14 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned > long *offsets, > > setup_vmalloc_vm_locked(vms[area], vas[area], VM_ALLOC, > pcpu_get_vm_areas); > + } > + spin_unlock(&vmap_area_lock); > > + /* populate the shadow space outside of the lock */ > + for (area = 0; area < nr_vms; area++) { > /* assume success here */ > kasan_populate_vmalloc(sizes[area], vms[area]); > } > - spin_unlock(&vmap_area_lock); > > kfree(vas); > return vms; > >
Re: [PATCH v11 1/4] kasan: support backing vmalloc space with real shadow memory
On 11/29/19 2:02 PM, Dmitry Vyukov wrote: > On Fri, Nov 29, 2019 at 11:58 AM Dmitry Vyukov wrote: >> >> On Fri, Nov 29, 2019 at 11:43 AM Dmitry Vyukov wrote: >>> >>> On Tue, Nov 19, 2019 at 10:54 AM Andrey Ryabinin >>> wrote: >>>> On 11/18/19 6:29 AM, Daniel Axtens wrote: >>>>> Qian Cai writes: >>>>> >>>>>> On Thu, 2019-10-31 at 20:39 +1100, Daniel Axtens wrote: >>>>>>> /* >>>>>>> * In this function, newly allocated vm_struct has VM_UNINITIALIZED >>>>>>> * flag. It means that vm_struct is not fully initialized. >>>>>>> @@ -3377,6 +3411,9 @@ struct vm_struct **pcpu_get_vm_areas(const >>>>>>> unsigned long *offsets, >>>>>>> >>>>>>> setup_vmalloc_vm_locked(vms[area], vas[area], VM_ALLOC, >>>>>>> pcpu_get_vm_areas); >>>>>>> + >>>>>>> + /* assume success here */ >>>>>>> + kasan_populate_vmalloc(sizes[area], vms[area]); >>>>>>> } >>>>>>> spin_unlock(&vmap_area_lock); >>>>>> >>>>>> Here it is all wrong. GFP_KERNEL with in_atomic(). >>>>> >>>>> I think this fix will work, I will do a v12 with it included. >>>> >>>> You can send just the fix. Andrew will fold it into the original patch >>>> before sending it to Linus. >>>> >>>> >>>> >>>>> diff --git a/mm/vmalloc.c b/mm/vmalloc.c >>>>> index a4b950a02d0b..bf030516258c 100644 >>>>> --- a/mm/vmalloc.c >>>>> +++ b/mm/vmalloc.c >>>>> @@ -3417,11 +3417,14 @@ struct vm_struct **pcpu_get_vm_areas(const >>>>> unsigned long *offsets, >>>>> >>>>> setup_vmalloc_vm_locked(vms[area], vas[area], VM_ALLOC, >>>>> pcpu_get_vm_areas); >>>>> + } >>>>> + spin_unlock(&vmap_area_lock); >>>>> >>>>> + /* populate the shadow space outside of the lock */ >>>>> + for (area = 0; area < nr_vms; area++) { >>>>> /* assume success here */ >>>>> kasan_populate_vmalloc(sizes[area], vms[area]); >>>>> } >>>>> - spin_unlock(&vmap_area_lock); >>>>> >>>>> kfree(vas); >>>>> return vms; >>> >>> Hi, >>> >>> I am testing this support on next-20191129 and seeing the following >>> warnings: >>> >>> BUG: sleeping function called from invalid context at mm/page_alloc.c:4681 >>> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 44, name: kworker/1:1 >>> 4 locks held by kworker/1:1/44: >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: >>> __write_once_size include/linux/compiler.h:247 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: >>> arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: atomic64_set >>> include/asm-generic/atomic-instrumented.h:868 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: >>> atomic_long_set include/asm-generic/atomic-long.h:40 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: set_work_data >>> kernel/workqueue.c:615 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: >>> set_work_pool_and_clear_pending kernel/workqueue.c:642 [inline] >>> #0: 888067c26d28 ((wq_completion)events){+.+.}, at: >>> process_one_work+0x88b/0x1750 kernel/workqueue.c:2235 >>> #1: c92afdf0 (pcpu_balance_work){+.+.}, at: >>> process_one_work+0x8c0/0x1750 kernel/workqueue.c:2239 >>> #2: 8943f080 (pcpu_alloc_mutex){+.+.}, at: >>> pcpu_balance_workfn+0xcc/0x13e0 mm/percpu.c:1845 >>> #3: 89450c78 (vmap_area_lock){+.+.}, at: spin_lock >>> include/linux/spinlock.h:338 [inline] >>> #3: 89450c78 (vmap_area_lock){+.+.}, at: >>> pcpu_get_vm_areas+0x1449/0x3df0 mm/vmalloc.c:3431 >>> Preemption disabled at: >>> [] spin_lock include/linux/spinlock.h:338 [inline] >>> [] pcpu_get_vm_areas+0x1449/0x3df0 mm/vmalloc.c:3431 >>> CPU: 1 PID: 44 Comm: kworker/1:1 Not
Re: [PATCH v11 1/4] kasan: support backing vmalloc space with real shadow memory
On 11/29/19 2:47 PM, Dmitry Vyukov wrote: > On Fri, Nov 29, 2019 at 12:38 PM Andrey Ryabinin > wrote: >>>>> >>>>> >>>>> Not sure if it's the same or not. Is it addressed by something in flight? >>>>> >>>>> My config: >>>>> https://gist.githubusercontent.com/dvyukov/36c7be311fdec9cd51c649f7c3cb2ddb/raw/39c6f864fdd0ffc53f0822b14c354a73c1695fa1/gistfile1.txt >>>> >>>> >>>> I've tried this fix for pcpu_get_vm_areas: >>>> https://groups.google.com/d/msg/kasan-dev/t_F2X1MWKwk/h152Z3q2AgAJ >>>> and it helps. But this will break syzbot on linux-next soon. >>> >>> >>> Can this be related as well? >>> Crashes on accesses to shadow on the ion memory... >> >> Nope, it's vm_map_ram() not being handled > > > Another suspicious one. Related to kasan/vmalloc? Very likely the same as with ion: # git grep vm_map_ram|grep xfs fs/xfs/xfs_buf.c:* vm_map_ram() will allocate auxiliary structures (e.g. fs/xfs/xfs_buf.c: bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count, > > BUG: unable to handle page fault for address: f52005b8 > #PF: supervisor read access in kernel mode > #PF: error_code(0x) - not-present page > PGD 7ffcd067 P4D 7ffcd067 PUD 2cd10067 PMD 66d76067 PTE 0 > Oops: [#1] PREEMPT SMP KASAN > CPU: 2 PID: 9211 Comm: syz-executor.2 Not tainted 5.4.0-next-20191129+ #6 > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS > rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014 > RIP: 0010:xfs_sb_read_verify+0xe9/0x540 fs/xfs/libxfs/xfs_sb.c:691 > Code: fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 1e 04 00 00 4d 8b ac 24 > 30 01 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 <0f> b6 > 04 02 84 c0 74 08 3c 03 0f 8e ad 03 00 00 41 8b 45 00 bf 58 > RSP: 0018:c9000a58f8d0 EFLAGS: 00010a06 > RAX: dc00 RBX: 1920014b1f1d RCX: c9000af42000 > RDX: 192005b8 RSI: 82914404 RDI: 88805cdb1460 > RBP: c9000a58fab0 R08: 8880610cd380 R09: ed1005a87045 > R10: ed1005a87044 R11: 88802d438223 R12: 88805cdb1340 > R13: c9002dc0 R14: c9000a58fa88 R15: 888061b5c000 > FS: 7fb49bda9700() GS:88802d40() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: f52005b8 CR3: 60769006 CR4: 00760ee0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > PKRU: 5554 > Call Trace: > xfs_buf_ioend+0x228/0xdc0 fs/xfs/xfs_buf.c:1162 > __xfs_buf_submit+0x38b/0xe50 fs/xfs/xfs_buf.c:1485 > xfs_buf_submit fs/xfs/xfs_buf.h:268 [inline] > xfs_buf_read_uncached+0x15c/0x560 fs/xfs/xfs_buf.c:897 > xfs_readsb+0x2d0/0x540 fs/xfs/xfs_mount.c:298 > xfs_fc_fill_super+0x3e6/0x11f0 fs/xfs/xfs_super.c:1415 > get_tree_bdev+0x444/0x620 fs/super.c:1340 > xfs_fc_get_tree+0x1c/0x20 fs/xfs/xfs_super.c:1550 > vfs_get_tree+0x8e/0x300 fs/super.c:1545 > do_new_mount fs/namespace.c:2822 [inline] > do_mount+0x152d/0x1b50 fs/namespace.c:3142 > ksys_mount+0x114/0x130 fs/namespace.c:3351 > __do_sys_mount fs/namespace.c:3365 [inline] > __se_sys_mount fs/namespace.c:3362 [inline] > __x64_sys_mount+0xbe/0x150 fs/namespace.c:3362 > do_syscall_64+0xfa/0x780 arch/x86/entry/common.c:294 > entry_SYSCALL_64_after_hwframe+0x49/0xbe > RIP: 0033:0x46736a > Code: 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f > 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d > 01 f0 ff ff 73 01 c3 48 c7 c1 bc ff ff ff f7 d8 64 89 01 48 > RSP: 002b:7fb49bda8a78 EFLAGS: 0202 ORIG_RAX: 00a5 > RAX: ffda RBX: 7fb49bda8af0 RCX: 0046736a > RDX: 7fb49bda8ad0 RSI: 2140 RDI: 7fb49bda8af0 > RBP: 7fb49bda8ad0 R08: 7fb49bda8b30 R09: 7fb49bda8ad0 > R10: R11: 0202 R12: 7fb49bda8b30 > R13: 004b1c60 R14: 004b006d R15: 7fb49bda96bc > Modules linked in: > Dumping ftrace buffer: >(ftrace buffer empty) > CR2: f52005b8 > ---[ end trace eddd8949d4c898df ]--- > RIP: 0010:xfs_sb_read_verify+0xe9/0x540 fs/xfs/libxfs/xfs_sb.c:691 > Code: fc ff df 48 c1 ea 03 80 3c 02 00 0f 85 1e 04 00 00 4d 8b ac 24 > 30 01 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 <0f> b6 > 04 02 84 c0 74 08 3c 03 0f 8e ad 03 00 00 41 8b 45 00 bf 58 > RSP: 0018:c9000a58f8d0 EFLAGS: 00010a06 > RAX: dc00 RBX: 1920014b1f1d RCX: c9000af42000 > RDX: 192005b8 RSI: 82914404 RD
Re: [RFC PATCH] lib: Introduce generic __cmpxchg_u64() and use it where needed
On 11/01/2018 07:32 PM, Peter Zijlstra wrote: > On Thu, Nov 01, 2018 at 03:22:15PM +, Trond Myklebust wrote: >> On Thu, 2018-11-01 at 15:59 +0100, Peter Zijlstra wrote: >>> On Thu, Nov 01, 2018 at 01:18:46PM +, Mark Rutland wrote: > > My one question (and the reason why I went with cmpxchg() in the > first place) would be about the overflow behaviour for > atomic_fetch_inc() and friends. I believe those functions should > be OK on x86, so that when we overflow the counter, it behaves > like an unsigned value and wraps back around. Is that the case > for all architectures? > > i.e. are atomic_t/atomic64_t always guaranteed to behave like > u32/u64 on increment? > > I could not find any documentation that explicitly stated that > they should. Peter, Will, I understand that the atomic_t/atomic64_t ops are required to wrap per 2's-complement. IIUC the refcount code relies on this. Can you confirm? >>> >>> There is quite a bit of core code that hard assumes 2s-complement. >>> Not only for atomics but for any signed integer type. Also see the >>> kernel using -fno-strict-overflow which implies -fwrapv, which >>> defines signed overflow to behave like 2s-complement (and rids us of >>> that particular UB). >> >> Fair enough, but there have also been bugfixes to explicitly fix unsafe >> C standards assumptions for signed integers. See, for instance commit >> 5a581b367b5d "jiffies: Avoid undefined behavior from signed overflow" >> from Paul McKenney. > > Yes, I feel Paul has been to too many C/C++ committee meetings and got > properly paranoid. Which isn't always a bad thing :-) > > But for us using -fno-strict-overflow which actually defines signed > overflow, I myself am really not worried. I'm also not sure if KASAN has > been taught about this, or if it will still (incorrectly) warn about UB > for signed types. > UBSAN warns about signed overflows despite -fno-strict-overflow if gcc version is < 8. I have learned recently that UBSAN in GCC 8 ignores signed overflows if -fno-strict-overflow of fwrapv is used. $ cat signed_overflow.c #include __attribute__((noinline)) int foo(int a, int b) { return a+b; } int main(void) { int a = 0x7fff; int b = 2; printf("%d\n", foo(a,b)); return 0; } $ gcc-8.2.0 -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int' -2147483647 $ gcc-8.2.0 -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out -2147483647 $ gcc-7.3.0 -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int' -2147483647 $ gcc-7.3.0 -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int' -2147483647 clang behaves the same way as GCC 8: $ clang -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out signed_overflow.c:6:10: runtime error: signed integer overflow: 2147483647 + 2 cannot be represented in type 'int' -2147483647 $ clang -fno-strict-overflow -fsanitize=signed-integer-overflow signed_overflow.c && ./a.out -2147483647 We can always just drop -fsanitize=signed-integer-overflow if it considered too noisy. Although it did catch some real bugs.
Re: [PATCH v3 3/3] powerpc/32: Add KASAN support
On 1/12/19 2:16 PM, Christophe Leroy wrote: > +KASAN_SANITIZE_early_32.o := n > +KASAN_SANITIZE_cputable.o := n > +KASAN_SANITIZE_prom_init.o := n > + Usually it's also good idea to disable branch profiling - define DISABLE_BRANCH_PROFILING either in top of these files or via Makefile. Branch profiling redefines if() statement and calls instrumented ftrace_likely_update in every if(). > diff --git a/arch/powerpc/mm/kasan_init.c b/arch/powerpc/mm/kasan_init.c > new file mode 100644 > index ..3edc9c2d2f3e > +void __init kasan_init(void) > +{ > + struct memblock_region *reg; > + > + for_each_memblock(memory, reg) > + kasan_init_region(reg); > + > + pr_info("KASAN init done\n"); Without "init_task.kasan_depth = 0;" kasan will not repot bugs. There is test_kasan module. Make sure that it produce reports.
Re: [PATCH v3 1/3] powerpc/mm: prepare kernel for KAsan on PPC32
On 1/15/19 2:14 PM, Dmitry Vyukov wrote: > On Tue, Jan 15, 2019 at 8:27 AM Christophe Leroy > wrote: >> On 01/14/2019 09:34 AM, Dmitry Vyukov wrote: >>> On Sat, Jan 12, 2019 at 12:16 PM Christophe Leroy >>> wrote: >>> > >>> > In kernel/cputable.c, explicitly use memcpy() in order >>> > to allow GCC to replace it with __memcpy() when KASAN is >>> > selected. >>> > >>> > Since commit 400c47d81ca38 ("powerpc32: memset: only use dcbz once >>> cache is >>> > enabled"), memset() can be used before activation of the cache, >>> > so no need to use memset_io() for zeroing the BSS. >>> > >>> > Signed-off-by: Christophe Leroy >>> > --- >>> > arch/powerpc/kernel/cputable.c | 4 ++-- >>> > arch/powerpc/kernel/setup_32.c | 6 ++ >>> > 2 files changed, 4 insertions(+), 6 deletions(-) >>> > >>> > diff --git a/arch/powerpc/kernel/cputable.c >>> b/arch/powerpc/kernel/cputable.c >>> > index 1eab54bc6ee9..84814c8d1bcb 100644 >>> > --- a/arch/powerpc/kernel/cputable.c >>> > +++ b/arch/powerpc/kernel/cputable.c >>> > @@ -2147,7 +2147,7 @@ void __init set_cur_cpu_spec(struct cpu_spec *s) >>> > struct cpu_spec *t = &the_cpu_spec; >>> > >>> > t = PTRRELOC(t); >>> > - *t = *s; >>> > + memcpy(t, s, sizeof(*t)); >>> >>> Hi Christophe, >>> >>> I understand why you are doing this, but this looks a bit fragile and >>> non-scalable. This may not work with the next version of compiler, >>> just different than yours version of compiler, clang, etc. >> >> My felling would be that this change makes it more solid. >> >> My understanding is that when you do *t = *s, the compiler can use >> whatever way it wants to do the copy. >> When you do memcpy(), you ensure it will do it that way and not another >> way, don't you ? > > It makes this single line more deterministic wrt code-gen (though, > strictly saying compiler can turn memcpy back into inlines > instructions, it knows memcpy semantics anyway). > But the problem I meant is that the set of places that are subject to > this problem is not deterministic. So if we go with this solution, > after this change it's in the status "works on your machine" and we > either need to commit to not using struct copies and zeroing > throughout kernel code or potentially have a long tail of other > similar cases, and since they can be triggered by another compiler > version, we may need to backport these changes to previous releases > too. Whereas if we would go with compiler flags, it would prevent the > problem in all current and future places and with other past/future > versions of compilers. > The patch will work for any compiler. The point of this patch is to make memcpy() visible to the preprocessor which will replace it with __memcpy(). After preprocessor's work, compiler will see just __memcpy() call here.
Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
On 2/11/19 3:25 PM, Andrey Konovalov wrote: > On Sat, Feb 9, 2019 at 12:55 PM christophe leroy > wrote: >> >> Hi Andrey, >> >> Le 08/02/2019 à 18:40, Andrey Konovalov a écrit : >>> On Fri, Feb 8, 2019 at 6:17 PM Christophe Leroy >>> wrote: Hi Daniel, Le 08/02/2019 à 17:18, Daniel Axtens a écrit : > Hi Christophe, > > I've been attempting to port this to 64-bit Book3e nohash (e6500), > although I think I've ended up with an approach more similar to Aneesh's > much earlier (2015) series for book3s. > > Part of this is just due to the changes between 32 and 64 bits - we need > to hack around the discontiguous mappings - but one thing that I'm > particularly puzzled by is what the kasan_early_init is supposed to do. It should be a problem as my patch uses a 'for_each_memblock(memory, reg)' loop. > >> +void __init kasan_early_init(void) >> +{ >> +unsigned long addr = KASAN_SHADOW_START; >> +unsigned long end = KASAN_SHADOW_END; >> +unsigned long next; >> +pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), addr); >> +int i; >> +phys_addr_t pa = __pa(kasan_early_shadow_page); >> + >> +BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK); >> + >> +if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE)) >> +panic("KASAN not supported with Hash MMU\n"); >> + >> +for (i = 0; i < PTRS_PER_PTE; i++) >> +__set_pte_at(&init_mm, (unsigned >> long)kasan_early_shadow_page, >> + kasan_early_shadow_pte + i, >> + pfn_pte(PHYS_PFN(pa), PAGE_KERNEL_RO), 0); >> + >> +do { >> +next = pgd_addr_end(addr, end); >> +pmd_populate_kernel(&init_mm, pmd, kasan_early_shadow_pte); >> +} while (pmd++, addr = next, addr != end); >> +} > > As far as I can tell it's mapping the early shadow page, read-only, over > the KASAN_SHADOW_START->KASAN_SHADOW_END range, and it's using the early > shadow PTE array from the generic code. > > I haven't been able to find an answer to why this is in the docs, so I > was wondering if you or anyone else could explain the early part of > kasan init a bit better. See https://www.kernel.org/doc/html/latest/dev-tools/kasan.html for an explanation of the shadow. When shadow is 0, it means the memory area is entirely accessible. It is necessary to setup a shadow area as soon as possible because all data accesses check the shadow area, from the begining (except for a few files where sanitizing has been disabled in Makefiles). Until the real shadow area is set, all access are granted thanks to the zero shadow area beeing for of zeros. >>> >>> Not entirely correct. kasan_early_init() indeed maps the whole shadow >>> memory range to the same kasan_early_shadow_page. However as kernel >>> loads and memory gets allocated this shadow page gets rewritten with >>> non-zero values by different KASAN allocator hooks. Since these values >>> come from completely different parts of the kernel, but all land on >>> the same page, kasan_early_shadow_page's content can be considered >>> garbage. When KASAN checks memory accesses for validity it detects >>> these garbage shadow values, but doesn't print any reports, as the >>> reporting routine bails out on the current->kasan_depth check (which >>> has the value of 1 initially). Only after kasan_init() completes, when >>> the proper shadow memory is mapped, current->kasan_depth gets set to 0 >>> and we start reporting bad accesses. >> >> That's surprising, because in the early phase I map the shadow area >> read-only, so I do not expect it to get modified unless RO protection is >> failing for some reason. > > Actually it might be that the allocator hooks don't modify shadow at > this point, as the allocator is not yet initialized. However stack > should be getting poisoned and unpoisoned from the very start. But the > generic statement that early shadow gets dirtied should be correct. > Might it be that you don't use stack instrumentation? > Yes, stack instrumentation is not used here, because shadow offset which we pass to the -fasan-shadow-offset= cflag is not specified here. So the logic in scrpits/Makefile.kasan just fallbacks to CFLAGS_KASAN_MINIMAL, which is outline and without stack instrumentation. Christophe, you can specify KASAN_SHADOW_OFFSET either in Kconfig (e.g. x86_64) or in Makefile (e.g. arm64). And make early mapping writable, because compiler generated code will write to shadow memory in function prologue/epilogue.
Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
On 2/12/19 4:08 AM, Daniel Axtens wrote: > Andrey Ryabinin writes: > >> On 2/11/19 3:25 PM, Andrey Konovalov wrote: >>> On Sat, Feb 9, 2019 at 12:55 PM christophe leroy >>> wrote: >>>> >>>> Hi Andrey, >>>> >>>> Le 08/02/2019 à 18:40, Andrey Konovalov a écrit : >>>>> On Fri, Feb 8, 2019 at 6:17 PM Christophe Leroy >>>>> wrote: >>>>>> >>>>>> Hi Daniel, >>>>>> >>>>>> Le 08/02/2019 à 17:18, Daniel Axtens a écrit : >>>>>>> Hi Christophe, >>>>>>> >>>>>>> I've been attempting to port this to 64-bit Book3e nohash (e6500), >>>>>>> although I think I've ended up with an approach more similar to Aneesh's >>>>>>> much earlier (2015) series for book3s. >>>>>>> >>>>>>> Part of this is just due to the changes between 32 and 64 bits - we need >>>>>>> to hack around the discontiguous mappings - but one thing that I'm >>>>>>> particularly puzzled by is what the kasan_early_init is supposed to do. >>>>>> >>>>>> It should be a problem as my patch uses a 'for_each_memblock(memory, >>>>>> reg)' loop. >>>>>> >>>>>>> >>>>>>>> +void __init kasan_early_init(void) >>>>>>>> +{ >>>>>>>> +unsigned long addr = KASAN_SHADOW_START; >>>>>>>> +unsigned long end = KASAN_SHADOW_END; >>>>>>>> +unsigned long next; >>>>>>>> +pmd_t *pmd = pmd_offset(pud_offset(pgd_offset_k(addr), addr), >>>>>>>> addr); >>>>>>>> +int i; >>>>>>>> +phys_addr_t pa = __pa(kasan_early_shadow_page); >>>>>>>> + >>>>>>>> +BUILD_BUG_ON(KASAN_SHADOW_START & ~PGDIR_MASK); >>>>>>>> + >>>>>>>> +if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE)) >>>>>>>> +panic("KASAN not supported with Hash MMU\n"); >>>>>>>> + >>>>>>>> +for (i = 0; i < PTRS_PER_PTE; i++) >>>>>>>> +__set_pte_at(&init_mm, (unsigned >>>>>>>> long)kasan_early_shadow_page, >>>>>>>> + kasan_early_shadow_pte + i, >>>>>>>> + pfn_pte(PHYS_PFN(pa), PAGE_KERNEL_RO), 0); >>>>>>>> + >>>>>>>> +do { >>>>>>>> +next = pgd_addr_end(addr, end); >>>>>>>> +pmd_populate_kernel(&init_mm, pmd, >>>>>>>> kasan_early_shadow_pte); >>>>>>>> +} while (pmd++, addr = next, addr != end); >>>>>>>> +} >>>>>>> >>>>>>> As far as I can tell it's mapping the early shadow page, read-only, over >>>>>>> the KASAN_SHADOW_START->KASAN_SHADOW_END range, and it's using the early >>>>>>> shadow PTE array from the generic code. >>>>>>> >>>>>>> I haven't been able to find an answer to why this is in the docs, so I >>>>>>> was wondering if you or anyone else could explain the early part of >>>>>>> kasan init a bit better. >>>>>> >>>>>> See https://www.kernel.org/doc/html/latest/dev-tools/kasan.html for an >>>>>> explanation of the shadow. >>>>>> >>>>>> When shadow is 0, it means the memory area is entirely accessible. >>>>>> >>>>>> It is necessary to setup a shadow area as soon as possible because all >>>>>> data accesses check the shadow area, from the begining (except for a few >>>>>> files where sanitizing has been disabled in Makefiles). >>>>>> >>>>>> Until the real shadow area is set, all access are granted thanks to the >>>>>> zero shadow area beeing for of zeros. >>>>> >>>>> Not entirely correct. kasan_early_init() indeed maps the whole shadow >>>>> memory range to the same kasan_early_shadow_page. However as kernel >>>>> loads and memory gets allocated this shadow page gets rewritten with >>>&g
Re: [PATCH v4 3/3] powerpc/32: Add KASAN support
On 2/12/19 2:38 PM, Christophe Leroy wrote: > > > Le 12/02/2019 à 02:08, Daniel Axtens a écrit : >> Andrey Ryabinin writes: >> >>> >>> Christophe, you can specify KASAN_SHADOW_OFFSET either in Kconfig (e.g. >>> x86_64) or >>> in Makefile (e.g. arm64). And make early mapping writable, because compiler >>> generated code will write >>> to shadow memory in function prologue/epilogue. >> >> Hmm. Is this limitation just that compilers have not implemented >> out-of-line support for stack instrumentation, or is there a deeper >> reason that stack/global instrumentation relies upon inline >> instrumentation? > > No, it looks like as soon as we define KASAN_SHADOW_OFFSET in Makefile in > addition to asm/kasan.h, stack instrumentation works with out-of-line. > I think you confusing two different things. CONFIG_KASAN_INLINE/CONFIG_KASAN_OUTLINE affects only generation of code that checks memory accesses, whether we call __asan_loadx()/__asan_storex() or directly insert code, checking shadow memory. But with stack instrumentation we also need to poison redzones around stack variables and unpoison them when we leave the function. That poisoning/unpoisoning thing is always inlined. Currently there is no option to make it out-of-line. So you can have stack instrumentation with KASAN_OUTLINE, but it just means that checking shadow memory on memory access will be outlined, poisoning/unpoisoing stack redzones will remain inlined.
Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
On 2/15/19 11:41 AM, Christophe Leroy wrote: > > > Le 14/02/2019 à 23:04, Daniel Axtens a écrit : >> Hi Christophe, >> >>> --- a/arch/powerpc/include/asm/string.h >>> +++ b/arch/powerpc/include/asm/string.h >>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void >>> *,__kernel_size_t); >>> extern void * memchr(const void *,int,__kernel_size_t); >>> extern void * memcpy_flushcache(void *,const void *,__kernel_size_t); >>> +void *__memset(void *s, int c, __kernel_size_t count); >>> +void *__memcpy(void *to, const void *from, __kernel_size_t n); >>> +void *__memmove(void *to, const void *from, __kernel_size_t n); >>> + >>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__) >>> +/* >>> + * For files that are not instrumented (e.g. mm/slub.c) we >>> + * should use not instrumented version of mem* functions. >>> + */ >>> +#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) >>> +#endif >>> + >> >> I'm finding that I miss tests like 'kasan test: kasan_memcmp >> out-of-bounds in memcmp' because the uninstrumented asm version is used >> instead of an instrumented C version. I ended up guarding the relevant >> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting >> the arch versions if we're not compiled with KASAN. >> >> I find I need to guard and unexport strncpy, strncmp, memchr and >> memcmp. Do you need to do this on 32bit as well, or are those tests >> passing anyway for some reason? > > Indeed, I didn't try the KASAN test module recently, because my configs don't > have CONFIG_MODULE by default. > > Trying to test it now, I am discovering that module loading oopses with > latest version of my series, I need to figure out exactly why. Here below the > oops by modprobing test_module (the one supposed to just say hello to the > world). > > What we see is an access to the RO kasan zero area. > > The shadow mem is 0xf7c0..0xffc0 > Linear kernel memory is shadowed by 0xf7c0-0xf8bf > 0xf8c0-0xffc0 is shadowed read only by the kasan zero page. > > Why is kasan trying to access that ? Isn't kasan supposed to not check stuff > in vmalloc area ? It tries to poison global variables in modules. If module is in vmalloc, than it will try to poison vmalloc. Given that the vmalloc area is not so big on 32bits, the easiest solution is to cover all vmalloc with RW shadow.
Re: [PATCH v5 3/3] powerpc/32: Add KASAN support
On 2/15/19 1:10 PM, Christophe Leroy wrote: > > > Le 15/02/2019 à 11:01, Andrey Ryabinin a écrit : >> >> >> On 2/15/19 11:41 AM, Christophe Leroy wrote: >>> >>> >>> Le 14/02/2019 à 23:04, Daniel Axtens a écrit : >>>> Hi Christophe, >>>> >>>>> --- a/arch/powerpc/include/asm/string.h >>>>> +++ b/arch/powerpc/include/asm/string.h >>>>> @@ -27,6 +27,20 @@ extern int memcmp(const void *,const void >>>>> *,__kernel_size_t); >>>>> extern void * memchr(const void *,int,__kernel_size_t); >>>>> extern void * memcpy_flushcache(void *,const void *,__kernel_size_t); >>>>> +void *__memset(void *s, int c, __kernel_size_t count); >>>>> +void *__memcpy(void *to, const void *from, __kernel_size_t n); >>>>> +void *__memmove(void *to, const void *from, __kernel_size_t n); >>>>> + >>>>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__) >>>>> +/* >>>>> + * For files that are not instrumented (e.g. mm/slub.c) we >>>>> + * should use not instrumented version of mem* functions. >>>>> + */ >>>>> +#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) >>>>> +#endif >>>>> + >>>> >>>> I'm finding that I miss tests like 'kasan test: kasan_memcmp >>>> out-of-bounds in memcmp' because the uninstrumented asm version is used >>>> instead of an instrumented C version. I ended up guarding the relevant >>>> __HAVE_ARCH_x symbols behind a #ifndef CONFIG_KASAN and only exporting >>>> the arch versions if we're not compiled with KASAN. >>>> >>>> I find I need to guard and unexport strncpy, strncmp, memchr and >>>> memcmp. Do you need to do this on 32bit as well, or are those tests >>>> passing anyway for some reason? >>> >>> Indeed, I didn't try the KASAN test module recently, because my configs >>> don't have CONFIG_MODULE by default. >>> >>> Trying to test it now, I am discovering that module loading oopses with >>> latest version of my series, I need to figure out exactly why. Here below >>> the oops by modprobing test_module (the one supposed to just say hello to >>> the world). >>> >>> What we see is an access to the RO kasan zero area. >>> >>> The shadow mem is 0xf7c0..0xffc0 >>> Linear kernel memory is shadowed by 0xf7c0-0xf8bf >>> 0xf8c0-0xffc0 is shadowed read only by the kasan zero page. >>> >>> Why is kasan trying to access that ? Isn't kasan supposed to not check >>> stuff in vmalloc area ? >> >> It tries to poison global variables in modules. If module is in vmalloc, >> than it will try to poison vmalloc. >> Given that the vmalloc area is not so big on 32bits, the easiest solution is >> to cover all vmalloc with RW shadow. >> > > Euh ... Not so big ? > > Memory: 96448K/131072K available (8016K kernel code, 1680K rwdata > , 2720K rodata, 624K init, 4678K bss, 34624K reserved, 0K cma-reserved) > Kernel virtual memory layout: > * 0xffefc000..0xc000 : fixmap > * 0xf7c0..0xffc0 : kasan shadow mem > * 0xf7a0..0xf7c0 : consistent mem > * 0xf7a0..0xf7a0 : early ioremap > * 0xc900..0xf7a0 : vmalloc & ioremap > > Here, vmalloc area size 0x2ea0, that is 746Mbytes. Shadow for this would > be 93Mbytes and we are already using 16Mbytes to shadow the linear memory > area this poor board has 128Mbytes RAM in total. > > So another solution is needed. > Ok. As a temporary workaround your can make __asan_register_globals() to skip globals in vmalloc(). Obviously it means that out-of-bounds accesses to in modules will be missed. Non temporary solution would making kasan to fully support vmalloc, i.e. remove RO shadow and allocate/free shadow on vmalloc()/vfree(). But this feels like separate task, out of scope of this patch set. It is also possible to follow some other arches - dedicate separate address range for modules, allocate/free shadow in module_alloc/free. But it doesn't seem worthy to implement this only for the sake of kasan, since vmalloc support needs to be done anyway.
Re: [PATCH v2 4/4] powerpc: Book3S 64-bit "heavyweight" KASAN support
On 12/11/19 5:24 PM, Daniel Axtens wrote: > Hi Balbir, > > +Discontiguous memory can occur when you have a machine with memory spread > +across multiple nodes. For example, on a Talos II with 64GB of RAM: > + > + - 32GB runs from 0x0 to 0x_0008__, > + - then there's a gap, > + - then the final 32GB runs from 0x_2000__ to > 0x_2008__ > + > +This can create _significant_ issues: > + > + - If we try to treat the machine as having 64GB of _contiguous_ RAM, we > would > + assume that ran from 0x0 to 0x_0010__. We'd then reserve > the > + last 1/8th - 0x_000e__ to 0x_0010__ as the > shadow > + region. But when we try to access any of that, we'll try to access > pages > + that are not physically present. > + If we reserved memory for KASAN from each node (discontig region), we might survive this no? May be we need NUMA aware KASAN? That might be a generic change, just thinking out loud. >>> >>> The challenge is that - AIUI - in inline instrumentation, the compiler >>> doesn't generate calls to things like __asan_loadN and >>> __asan_storeN. Instead it uses -fasan-shadow-offset to compute the >>> checks, and only calls the __asan_report* family of functions if it >>> detects an issue. This also matches what I can observe with objdump >>> across outline and inline instrumentation settings. >>> >>> This means that for this sort of thing to work we would need to either >>> drop back to out-of-line calls, or teach the compiler how to use a >>> nonlinear, NUMA aware mem-to-shadow mapping. >> >> Yes, out of line is expensive, but seems to work well for all use cases. > > I'm not sure this is true. Looking at scripts/Makefile.kasan, allocas, > stacks and globals will only be instrumented if you can provide > KASAN_SHADOW_OFFSET. In the case you're proposing, we can't provide a > static offset. I _think_ this is a compiler limitation, where some of > those instrumentations only work/make sense with a static offset, but > perhaps that's not right? Dmitry and Andrey, can you shed some light on > this? > There is no code in the kernel is poisoning/unpoisoning redzones/variables on stack. It's because it's always done by the compiler, it inserts some code in prologue/epilogue of every function. So compiler needs to know the shadow offset which will be used to poison/unpoison stack frames. There is no such kind of limitation on globals instrumentation. The only reason globals instrumentation depends on -fasan-shadow-offset is because there was some bug related to globals in old gcc version which didn't support -fasan-shadow-offset. If you want stack instrumentation with not standard mem-to-shadow mapping, the options are: 1. Patch compiler to make it possible the poisoning/unpoisonig of stack frames via function calls. 2. Use out-line instrumentation and do whatever mem-to-shadow mapping you want, but keep all kernel stacks in some special place for which standard mem-to-shadow mapping (addr >>3 +offset) works. > Also, as it currently stands, the speed difference between inline and > outline is approximately 2x, and given that we'd like to run this > full-time in syzkaller I think there is value in trading off speed for > some limitations. >
Re: [PATCH v8 1/5] kasan: support backing vmalloc space with real shadow memory
On 10/1/19 9:58 AM, Daniel Axtens wrote: > core_initcall(kasan_memhotplug_init); > #endif > + > +#ifdef CONFIG_KASAN_VMALLOC > +static int kasan_populate_vmalloc_pte(pte_t *ptep, unsigned long addr, > + void *unused) > +{ > + unsigned long page; > + pte_t pte; > + > + if (likely(!pte_none(*ptep))) > + return 0; > + > + page = __get_free_page(GFP_KERNEL); > + if (!page) > + return -ENOMEM; > + > + memset((void *)page, KASAN_VMALLOC_INVALID, PAGE_SIZE); > + pte = pfn_pte(PFN_DOWN(__pa(page)), PAGE_KERNEL); > + > + /* > + * Ensure poisoning is visible before the shadow is made visible > + * to other CPUs. > + */ > + smp_wmb(); I'm not quite understand what this barrier do and why it needed. And if it's really needed there should be a pairing barrier on the other side which I don't see. > + > + spin_lock(&init_mm.page_table_lock); > + if (likely(pte_none(*ptep))) { > + set_pte_at(&init_mm, addr, ptep, pte); > + page = 0; > + } > + spin_unlock(&init_mm.page_table_lock); > + if (page) > + free_page(page); > + return 0; > +} > + ... > @@ -754,6 +769,8 @@ merge_or_add_vmap_area(struct vmap_area *va, > } > > insert: > + kasan_release_vmalloc(orig_start, orig_end, va->va_start, va->va_end); > + > if (!merged) { > link_va(va, root, parent, link, head); > augment_tree_propagate_from(va); > @@ -2068,6 +2085,22 @@ static struct vm_struct *__get_vm_area_node(unsigned > long size, > > setup_vmalloc_vm(area, va, flags, caller); > > + /* > + * For KASAN, if we are in vmalloc space, we need to cover the shadow > + * area with real memory. If we come here through VM_ALLOC, this is > + * done by a higher level function that has access to the true size, > + * which might not be a full page. > + * > + * We assume module space comes via VM_ALLOC path. > + */ > + if (is_vmalloc_addr(area->addr) && !(area->flags & VM_ALLOC)) { > + if (kasan_populate_vmalloc(area->size, area)) { > + unmap_vmap_area(va); > + kfree(area); > + return NULL; > + } > + } > + > return area; > } > > @@ -2245,6 +2278,9 @@ static void __vunmap(const void *addr, int > deallocate_pages) > debug_check_no_locks_freed(area->addr, get_vm_area_size(area)); > debug_check_no_obj_freed(area->addr, get_vm_area_size(area)); > > + if (area->flags & VM_KASAN) > + kasan_poison_vmalloc(area->addr, area->size); > + > vm_remove_mappings(area, deallocate_pages); > > if (deallocate_pages) { > @@ -2497,6 +2533,9 @@ void *__vmalloc_node_range(unsigned long size, unsigned > long align, > if (!addr) > return NULL; > > + if (kasan_populate_vmalloc(real_size, area)) > + return NULL; > + KASAN itself uses __vmalloc_node_range() to allocate and map shadow in memory online callback. So we should either skip non-vmalloc and non-module addresses here or teach kasan's memory online/offline callbacks to not use __vmalloc_node_range() (do something similar to kasan_populate_vmalloc() perhaps?).
Re: [PATCH v8 1/5] kasan: support backing vmalloc space with real shadow memory
On 10/14/19 4:57 PM, Daniel Axtens wrote: > Hi Andrey, > > >>> + /* >>> +* Ensure poisoning is visible before the shadow is made visible >>> +* to other CPUs. >>> +*/ >>> + smp_wmb(); >> >> I'm not quite understand what this barrier do and why it needed. >> And if it's really needed there should be a pairing barrier >> on the other side which I don't see. > > Mark might be better able to answer this, but my understanding is that > we want to make sure that we never have a situation where the writes are > reordered so that PTE is installed before all the poisioning is written > out. I think it follows the logic in __pte_alloc() in mm/memory.c: > > /* >* Ensure all pte setup (eg. pte page lock and page clearing) are >* visible before the pte is made visible to other CPUs by being >* put into page tables. >* >* The other side of the story is the pointer chasing in the page >* table walking code (when walking the page table without locking; >* ie. most of the time). Fortunately, these data accesses consist >* of a chain of data-dependent loads, meaning most CPUs (alpha >* being the notable exception) will already guarantee loads are >* seen in-order. See the alpha page table accessors for the >* smp_read_barrier_depends() barriers in page table walking code. >*/ > smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */ > > I can clarify the comment. > I don't see how is this relevant here. barrier in __pte_alloc() for very the following case: CPU 0 CPU 1 __pte_alloc(): pte_offset_kernel(pmd_t * dir, unsigned long address): pgtable_t new = pte_alloc_one(mm);pte_t *new = (pte_t *) pmd_page_vaddr(*dir) + ((address >> PAGE_SHIFT) & (PTRS_PER_PAGE - 1)); smp_wmb(); smp_read_barrier_depends(); pmd_populate(mm, pmd, new); /* do something with pte, e.g. check if (pte_none(*new)) */ It's needed to ensure that if CPU1 sees pmd_populate() it also sees initialized contents of the 'new'. In our case the barrier would have been needed if we had the other side like this: if (!pte_none(*vmalloc_shadow_pte)) { shadow_addr = (unsigned long)__va(pte_pfn(*vmalloc_shadow_pte) << PAGE_SHIFT); smp_read_barrier_depends(); *shadow_addr; /* read the shadow, barrier ensures that if we see installed pte, we will see initialized shadow memory. */ } Without such other side the barrier is pointless.
Re: [PATCH v8 1/5] kasan: support backing vmalloc space with real shadow memory
On 10/16/19 4:22 PM, Mark Rutland wrote: > Hi Andrey, > > On Wed, Oct 16, 2019 at 03:19:50PM +0300, Andrey Ryabinin wrote: >> On 10/14/19 4:57 PM, Daniel Axtens wrote: >>>>> + /* >>>>> + * Ensure poisoning is visible before the shadow is made visible >>>>> + * to other CPUs. >>>>> + */ >>>>> + smp_wmb(); >>>> >>>> I'm not quite understand what this barrier do and why it needed. >>>> And if it's really needed there should be a pairing barrier >>>> on the other side which I don't see. >>> >>> Mark might be better able to answer this, but my understanding is that >>> we want to make sure that we never have a situation where the writes are >>> reordered so that PTE is installed before all the poisioning is written >>> out. I think it follows the logic in __pte_alloc() in mm/memory.c: >>> >>> /* >>> * Ensure all pte setup (eg. pte page lock and page clearing) are >>> * visible before the pte is made visible to other CPUs by being >>> * put into page tables. >>> * >>> * The other side of the story is the pointer chasing in the page >>> * table walking code (when walking the page table without locking; >>> * ie. most of the time). Fortunately, these data accesses consist >>> * of a chain of data-dependent loads, meaning most CPUs (alpha >>> * being the notable exception) will already guarantee loads are >>> * seen in-order. See the alpha page table accessors for the >>> * smp_read_barrier_depends() barriers in page table walking code. >>> */ >>> smp_wmb(); /* Could be smp_wmb__xxx(before|after)_spin_lock */ >>> >>> I can clarify the comment. >> >> I don't see how is this relevant here. > > The problem isn't quite the same, but it's a similar shape. See below > for more details. > >> barrier in __pte_alloc() for very the following case: >> >> CPU 0CPU 1 >> __pte_alloc(): >> pte_offset_kernel(pmd_t * dir, unsigned long address): >> pgtable_t new = pte_alloc_one(mm);pte_t *new = >> (pte_t *) pmd_page_vaddr(*dir) + ((address >> PAGE_SHIFT) & (PTRS_PER_PAGE - >> 1)); >> smp_wmb(); >> smp_read_barrier_depends(); >> pmd_populate(mm, pmd, new); >> /* do something with >> pte, e.g. check if (pte_none(*new)) */ >> >> >> It's needed to ensure that if CPU1 sees pmd_populate() it also sees >> initialized contents of the 'new'. >> >> In our case the barrier would have been needed if we had the other side like >> this: >> >> if (!pte_none(*vmalloc_shadow_pte)) { >> shadow_addr = (unsigned long)__va(pte_pfn(*vmalloc_shadow_pte) << >> PAGE_SHIFT); >> smp_read_barrier_depends(); >> *shadow_addr; /* read the shadow, barrier ensures that if we see >> installed pte, we will see initialized shadow memory. */ >> } >> >> >> Without such other side the barrier is pointless. > > The barrier isn't pointless, but we are relying on a subtlety that is > not captured in LKMM, as one of the observers involved is the TLB (and > associated page table walkers) of the CPU. > > Until the PTE written by CPU 0 has been observed by the TLB of CPU 1, it > is not possible for CPU 1 to satisfy loads from the memory that PTE > maps, as it doesn't yet know which memory that is. > > Once the PTE written by CPU has been observed by the TLB of CPU 1, it is > possible for CPU 1 to satisfy those loads. At this instant, CPU 1 must > respect the smp_wmb() before the PTE was written, and hence sees zeroes s/zeroes/poison values > written before this. Note that if this were not true, we could not > safely swap userspace memory. > > There is the risk (as laid out in [1]) that CPU 1 attempts to hoist the > loads of the shadow memory above the load of the PTE, samples a stale > (faulting) status from the TLB, then performs the load of the PTE and > sees a valid value. In this case (on arm64) a spurious fault could be > taken when the access is architecturally performed. > > It is possible on arm64 to use a barrier here to prevent the spurious > fault, but this is not smp_read_barrier_depe
Re: [PATCH v12 07/11] x86/kasan: add and use kasan_map_populate()
On 10/18/2017 08:14 PM, Pavel Tatashin wrote: > Thank you Andrey, I will test this patch. Should it go on top or replace the > existing patch in mm-tree? ARM and x86 should be done the same either both as > follow-ups or both replace. > It's a replacement of your patch. > Pavel > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majord...@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Don't email: mailto:"d...@kvack.org";> em...@kvack.org
Re: llist code relies on undefined behaviour, upsets llvm/clang
2017-01-16 15:53 GMT+03:00 Peter Zijlstra : > On Mon, Jan 16, 2017 at 10:42:29PM +1100, Anton Blanchard wrote: >> Hi Peter, >> >> > Last I checked I couldn't build a x86_64 kernel with llvm. So no, not >> > something I've ever ran into. >> > >> > Also, I would argue that this is broken in llvm, the kernel very much >> > relies on things like this all over the place. Sure, we're way outside >> > of what the C language spec says, but who bloody cares ;-) >> >> True, but is there anything preventing gcc from implementing this >> optimisation in the future? If we are relying on undefined behaviour we >> should have a -fno-strict-* option to cover it. >> >> > If llvm wants to compile the kernel, it needs to learn the C dialect >> > the kernel uses. >> >> LLVM has done that before (eg adding -fno-strict-overflow). I don't >> think that option covers this case however. > > Our comment there states: > > # disable invalid "can't wrap" optimizations for signed / pointers > KBUILD_CFLAGS += $(call cc-option,-fno-strict-overflow) > > So this option should apply to pointer arithmetic, therefore I would > expect -fno-strict-overflow to actually apply here, or am I missing > something? That case is null pointer check optimization. '->member' has non-zero offset in struct, so LLVM assumes that pos->member != NULL and optimize away this check. LLVM/clang currently doesn't have -fno-delete-null-pointer-checks
Re: [RFC PATCH v1] powerpc/radix/kasan: KASAN support for Radix
On 07/29/2017 05:09 PM, Balbir Singh wrote: > This is the first attempt to implement KASAN for radix > on powerpc64. Aneesh Kumar implemented KASAN for hash 64 > in limited mode (support only for kernel linear mapping) > (https://lwn.net/Articles/655642/) > > This patch does the following: > 1. Defines its own zero_page,pte,pmd and pud because > the generic PTRS_PER_PTE, etc are variables on ppc64 > book3s. Since the implementation is for radix, we use > the radix constants. This patch uses ARCH_DEFINES_KASAN_ZERO_PTE > for that purpose > 2. There is a new function check_return_arch_not_ready() > which is defined for ppc64/book3s/radix and overrides the > checks in check_memory_region_inline() until the arch has > done kasan setup is done for the architecture. This is needed > for powerpc. A lot of functions are called in real mode prior > to MMU paging init, we could fix some of this by using > the kasan_early_init() bits, but that just maps the zero > page and does not do useful reporting. For this RFC we > just delay the checks in mem* functions till kasan_init() check_return_arch_not_ready() works only for outline instrumentation and without stack instrumentation. I guess this works for you only because CONFIG_KASAN_SHADOW_OFFSET is not defined. Therefore test for CFLAGS_KASAN can't pass, as '-fasan-shadow-offset= ' is invalid option, so CFLAGS_KASAN_MINIMAL is used instead. Or maybe you just used gcc 4.9.x which don't have full kasan support. This is also the reason why some tests doesn't pass for you. For stack instrumentation you'll have to implement kasan_early_init() and define CONFIG_KASAN_SHADOW_OFFSET. > 3. This patch renames memcpy/memset/memmove to their > equivalent __memcpy/__memset/__memmove and for files > that skip KASAN via KASAN_SANITIZE, we use the __ > variants. This is largely based on Aneesh's patchset > mentioned above > 4. In paca.c, some explicit memcpy inserted by the > compiler/linker is replaced via explicit memcpy > for structure content copying > 5. prom_init and a few other files have KASAN_SANITIZE > set to n, I think with the delayed checks (#2 above) > we might be able to work around many of them > 6. Resizing of virtual address space is done a little > aggressively the size is reduced to 1/4 and totally > to 1/2. For the RFC it was considered OK, since this > is just a debug tool for developers. This can be revisited > in the final implementation > > Tests: > > I ran test_kasan.ko and it reported errors for all test > cases except for > > kasan test: memcg_accounted_kmem_cache allocate memcg accounted object > kasan test: kasan_stack_oob out-of-bounds on stack > kasan test: kasan_global_oob out-of-bounds global variable > kasan test: use_after_scope_test use-after-scope on int > kasan test: use_after_scope_test use-after-scope on array > > Based on my understanding of the test, which is an expected > kasan bug report after each test starting with a "===" line. > Right, with exception of memc_accounted_kmem_cache test. The rest are expected to produce the kasan report unless CLFAGS_KASAN_MINIMAL used. use_after_scope tests also require fresh gcc 7.
Re: [PATCH v2 1/3] kasan: Avoid sleepable page allocation from atomic context
On 4/9/25 4:25 PM, Alexander Gordeev wrote: > On Wed, Apr 09, 2025 at 04:10:58PM +0200, Andrey Ryabinin wrote: > > Hi Andrey, > >>> @@ -301,7 +301,7 @@ static int kasan_populate_vmalloc_pte(pte_t *ptep, >>> unsigned long addr, >>> if (likely(!pte_none(ptep_get(ptep >>> return 0; >>> >>> - page = __get_free_page(GFP_KERNEL); >>> + page = __get_free_page(GFP_ATOMIC); >>> if (!page) >>> return -ENOMEM; >>> >> >> I think a better way to fix this would be moving out allocation from atomic >> context. Allocate page prior >> to apply_to_page_range() call and pass it down to >> kasan_populate_vmalloc_pte(). > > I think the page address could be passed as the parameter to > kasan_populate_vmalloc_pte(). We'll need to pass it as 'struct page **page' or maybe as pointer to some struct, e.g.: struct page_data { struct page *page; }; So, the kasan_populate_vmalloc_pte() would do something like this: kasan_populate_vmalloc_pte() { if (!pte_none) return 0; if (!page_data->page) return -EAGAIN; //use page to set pte //NULLify pointer so that next kasan_populate_vmalloc_pte() will bail // out to allocate new page page_data->page = NULL; } And it might be good idea to add 'last_addr' to page_data, so that we know where we stopped so that the next apply_to_page_range() call could continue, instead of starting from the beginning. > >> Whenever kasan_populate_vmalloc_pte() will require additional page we could >> bail out with -EAGAIN, >> and allocate another one. > > When would it be needed? kasan_populate_vmalloc_pte() handles just one page. > apply_to_page_range() goes over range of addresses and calls kasan_populate_vmalloc_pte() multiple times (each time with different 'addr' but the same '*unused' arg). Things will go wrong if you'll use same page multiple times for different addresses. > Thanks!
Re: [PATCH v2 1/3] kasan: Avoid sleepable page allocation from atomic context
On 4/8/25 6:07 PM, Alexander Gordeev wrote: > apply_to_page_range() enters lazy MMU mode and then invokes > kasan_populate_vmalloc_pte() callback on each page table walk > iteration. The lazy MMU mode may only be entered only under > protection of the page table lock. However, the callback can > go into sleep when trying to allocate a single page. > > Change __get_free_page() allocation mode from GFP_KERNEL to > GFP_ATOMIC to avoid scheduling out while in atomic context. > > Cc: sta...@vger.kernel.org > Fixes: 3c5c3cfb9ef4 ("kasan: support backing vmalloc space with real shadow > memory") > Signed-off-by: Alexander Gordeev > --- > mm/kasan/shadow.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c > index 88d1c9dcb507..edfa77959474 100644 > --- a/mm/kasan/shadow.c > +++ b/mm/kasan/shadow.c > @@ -301,7 +301,7 @@ static int kasan_populate_vmalloc_pte(pte_t *ptep, > unsigned long addr, > if (likely(!pte_none(ptep_get(ptep > return 0; > > - page = __get_free_page(GFP_KERNEL); > + page = __get_free_page(GFP_ATOMIC); > if (!page) > return -ENOMEM; > I think a better way to fix this would be moving out allocation from atomic context. Allocate page prior to apply_to_page_range() call and pass it down to kasan_populate_vmalloc_pte(). Whenever kasan_populate_vmalloc_pte() will require additional page we could bail out with -EAGAIN, and allocate another one.
Re: [PATCH v3 01/12] lib/kasan: introduce CONFIG_ARCH_DEFER_KASAN option
On 7/18/25 12:10 AM, Andrew Morton wrote: > On Thu, 17 Jul 2025 19:27:21 +0500 Sabyrzhan Tasbolatov > wrote: > >> Introduce CONFIG_ARCH_DEFER_KASAN to identify architectures that need >> to defer KASAN initialization until shadow memory is properly set up. >> >> Some architectures (like PowerPC with radix MMU) need to set up their >> shadow memory mappings before KASAN can be safely enabled, while others >> (like s390, x86, arm) can enable KASAN much earlier or even from the >> beginning. >> >> This option allows us to: >> 1. Use static keys only where needed (avoiding overhead) >> 2. Use compile-time constants for arch that don't need runtime checks >> 3. Maintain optimal performance for both scenarios >> >> Architectures that need deferred KASAN should select this option. >> Architectures that can enable KASAN early will get compile-time >> optimizations instead of runtime checks. > > Looks nice and appears quite mature. I'm reluctant to add it to mm.git > during -rc6, especially given the lack of formal review and ack tags. > > But but but, that's what the mm-new branch is for. I guess I'll add it > to get some additional exposure, but whether I'll advance it into > mm-unstable/linux-next for this cycle is unclear. > > What do you (and others) think? After looking a bit, it breaks UM and probably LoongArch too. I'd say it needs more work and not ready even for mm-new.
Re: [PATCH v3 08/12] kasan/um: select ARCH_DEFER_KASAN and call kasan_init_generic
On 7/22/25 4:17 PM, Sabyrzhan Tasbolatov wrote: > On Tue, Jul 22, 2025 at 4:00 AM Andrey Ryabinin > wrote: >> >> >> >> On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: >>> UserMode Linux needs deferred KASAN initialization as it has a custom >>> kasan_arch_is_ready() implementation that tracks shadow memory readiness >>> via the kasan_um_is_ready flag. >>> >>> Select ARCH_DEFER_KASAN to enable the unified static key mechanism >>> for runtime KASAN control. Call kasan_init_generic() which handles >>> Generic KASAN initialization and enables the static key. >>> >>> Delete the key kasan_um_is_ready in favor of the unified kasan_enabled() >>> interface. >>> >>> Note that kasan_init_generic has __init macro, which is called by >>> kasan_init() which is not marked with __init in arch/um code. >>> >>> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 >>> Signed-off-by: Sabyrzhan Tasbolatov >>> --- >>> Changes in v3: >>> - Added CONFIG_ARCH_DEFER_KASAN selection for proper runtime control >>> --- >>> arch/um/Kconfig | 1 + >>> arch/um/include/asm/kasan.h | 5 - >>> arch/um/kernel/mem.c| 4 ++-- >>> 3 files changed, 3 insertions(+), 7 deletions(-) >>> >>> diff --git a/arch/um/Kconfig b/arch/um/Kconfig >>> index f08e8a7fac9..fd6d78bba52 100644 >>> --- a/arch/um/Kconfig >>> +++ b/arch/um/Kconfig >>> @@ -8,6 +8,7 @@ config UML >>> select ARCH_WANTS_DYNAMIC_TASK_STRUCT >>> select ARCH_HAS_CPU_FINALIZE_INIT >>> select ARCH_HAS_FORTIFY_SOURCE >>> + select ARCH_DEFER_KASAN >>> select ARCH_HAS_GCOV_PROFILE_ALL >>> select ARCH_HAS_KCOV >>> select ARCH_HAS_STRNCPY_FROM_USER >>> diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h >>> index f97bb1f7b85..81bcdc0f962 100644 >>> --- a/arch/um/include/asm/kasan.h >>> +++ b/arch/um/include/asm/kasan.h >>> @@ -24,11 +24,6 @@ >>> >>> #ifdef CONFIG_KASAN >>> void kasan_init(void); >>> -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 */ >>> diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c >>> index 76bec7de81b..058cb70e330 100644 >>> --- a/arch/um/kernel/mem.c >>> +++ b/arch/um/kernel/mem.c >>> @@ -21,9 +21,9 @@ >>> #include >>> #include >>> #include >>> +#include >>> >>> #ifdef CONFIG_KASAN >>> -int kasan_um_is_ready; >>> void kasan_init(void) >>> { >>> /* >>> @@ -32,7 +32,7 @@ void kasan_init(void) >>>*/ >>> kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); >>> init_task.kasan_depth = 0; >>> - kasan_um_is_ready = true; >>> + kasan_init_generic(); >> >> I think this runs before jump_label_init(), and static keys shouldn't be >> switched before that.> } > > I got the warning in my local compilation and from kernel CI [1]. > > arch/um places kasan_init() in own `.kasan_init` section, while > kasan_init_generic() is called from __init. No, kasan_init() is in text section as the warning says. It's kasan_init_ptr in .kasan_init. Adding __init to kasan_init() should fix the warning. > Could you suggest a way how I can verify the functions call order? > By code inspection? or run uder gdb. kasan_init() is initialization routine called before main(). jump_label_init() called from start_kernel()<-start_kernel_proc()<-... main() > I need to familiarize myself with how to run arch/um locally It's as simple as: ARCH=um make ./linux rootfstype=hostfs ro init=/bin/bash
Re: [PATCH v3 00/12] kasan: unify kasan_arch_is_ready() and remove arch-specific implementations
On 7/22/25 8:21 PM, Sabyrzhan Tasbolatov wrote: > On Tue, Jul 22, 2025 at 3:59 AM Andrey Ryabinin > wrote: >> >> >> >> On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: >> >>> === Testing with patches >>> >>> Testing in v3: >>> >>> - Compiled every affected arch with no errors: >>> >>> $ make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \ >>> OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \ >>> HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld \ >>> ARCH=$ARCH >>> >>> $ clang --version >>> ClangBuiltLinux clang version 19.1.4 >>> Target: x86_64-unknown-linux-gnu >>> Thread model: posix >>> >>> - make ARCH=um produces the warning during compiling: >>> MODPOST Module.symvers >>> WARNING: modpost: vmlinux: section mismatch in reference: \ >>> kasan_init+0x43 (section: .ltext) -> \ >>> kasan_init_generic (section: .init.text) >>> >>> AFAIU, it's due to the code in arch/um/kernel/mem.c, where kasan_init() >>> is placed in own section ".kasan_init", which calls kasan_init_generic() >>> which is marked with "__init". >>> >>> - Booting via qemu-system- and running KUnit tests: >>> >>> * arm64 (GENERIC, HW_TAGS, SW_TAGS): no regression, same above results. >>> * x86_64 (GENERIC): no regression, no errors >>> >> >> It would be interesting to see whether ARCH_DEFER_KASAN=y arches work. >> These series add static key into __asan_load*()/_store*() which are called >> from everywhere, including the code patching static branches during the >> switch. >> >> I have suspicion that the code patching static branches during static key >> switch >> might not be prepared to the fact the current CPU might try to execute this >> static >> branch in the middle of switch. > > AFAIU, you're referring to this function in mm/kasan/generic.c: > > static __always_inline bool check_region_inline(const void *addr, > > size_t size, bool write, > > unsigned long ret_ip) > { > if (!kasan_shadow_initialized()) > return true; > ... > } > > and particularly, to architectures that selects ARCH_DEFER_KASAN=y, which are > loongarch, powerpc, um. So when these arch try to enable the static key: > > 1. static_branch_enable(&kasan_flag_enabled) called > 2. Kernel patches code - changes jump instructions > 3. Code patching involves memory writes > 4. Memory writes can trigger any KASAN wrapper function > 5. Wrapper calls kasan_shadow_initialized() > 6. kasan_shadow_initialized() calls static_branch_likely(&kasan_flag_enabled) > 7. This reads the static key being patched --- this is the potential issue? > Yes, that's right. > The current runtime check is following in tis v3 patch series: > > #ifdef CONFIG_ARCH_DEFER_KASAN > ... > static __always_inline bool kasan_shadow_initialized(void) > { > return static_branch_likely(&kasan_flag_enabled); > } > ... > #endif > > I wonder, if I should add some protection only for KASAN_GENERIC, > where check_region_inline() is called (or for all KASAN modes?): > > #ifdef CONFIG_ARCH_DEFER_KASAN > ... > static __always_inline bool kasan_shadow_initialized(void) > { > /* Avoid recursion (?) during static key patching */ > if (static_key_count(&kasan_flag_enabled.key) < 0) > return false; > return static_branch_likely(&kasan_flag_enabled); > } > ... > #endif > > Please suggest where the issue is and if I understood the problem. I don't know if it's a real problem or not. I'm just pointing out that we might have tricky use case here and maybe that's a problem, because nobody had such use case in mind. But maybe it's just fine. I think we just need to boot test it, to see if this works. > I might try to run QEMU on powerpc with KUnits to see if I see any logs. powerpc used static key same way before your patches, so powerpc should be fine.
Re: [PATCH v3 01/12] lib/kasan: introduce CONFIG_ARCH_DEFER_KASAN option
On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: > Introduce CONFIG_ARCH_DEFER_KASAN to identify architectures that need > to defer KASAN initialization until shadow memory is properly set up. > > Some architectures (like PowerPC with radix MMU) need to set up their > shadow memory mappings before KASAN can be safely enabled, while others > (like s390, x86, arm) can enable KASAN much earlier or even from the > beginning. > > This option allows us to: > 1. Use static keys only where needed (avoiding overhead) > 2. Use compile-time constants for arch that don't need runtime checks > 3. Maintain optimal performance for both scenarios > > Architectures that need deferred KASAN should select this option. > Architectures that can enable KASAN early will get compile-time > optimizations instead of runtime checks. > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 > Signed-off-by: Sabyrzhan Tasbolatov > --- > Changes in v3: > - Introduced CONFIG_ARCH_DEFER_KASAN to control static key usage > --- > lib/Kconfig.kasan | 8 > 1 file changed, 8 insertions(+) > > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index f82889a830f..38456560c85 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -19,6 +19,14 @@ config ARCH_DISABLE_KASAN_INLINE > Disables both inline and stack instrumentation. Selected by > architectures that do not support these instrumentation types. > > +config ARCH_DEFER_KASAN > + bool > + help > + Architectures should select this if they need to defer KASAN > + initialization until shadow memory is properly set up. This > + enables runtime control via static keys. Otherwise, KASAN uses > + compile-time constants for better performance. > + > config CC_HAS_KASAN_GENERIC > def_bool $(cc-option, -fsanitize=kernel-address) > This needs to be merged with the next patch where this option at least has some users.
Re: [PATCH v3 02/12] kasan: unify static kasan_flag_enabled across modes
On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: > Historically, the runtime static key kasan_flag_enabled existed only for > CONFIG_KASAN_HW_TAGS mode. Generic and SW_TAGS modes either relied on > architecture-specific kasan_arch_is_ready() implementations or evaluated > KASAN checks unconditionally, leading to code duplication. > > This patch implements two-level approach: > > 1. kasan_enabled() - controls if KASAN is enabled at all (compile-time) > 2. kasan_shadow_initialized() - tracks shadow memory >initialization (runtime) > > For architectures that select ARCH_DEFER_KASAN: kasan_shadow_initialized() > uses a static key that gets enabled when shadow memory is ready. > > For architectures that don't: kasan_shadow_initialized() returns > IS_ENABLED(CONFIG_KASAN) since shadow is ready from the start. > > This provides: > - Consistent interface across all KASAN modes > - Runtime control only where actually needed > - Compile-time constants for optimal performance where possible > - Clear separation between "KASAN configured" vs "shadow ready" > > Also adds kasan_init_generic() function that enables the shadow flag and > handles initialization for Generic mode, and updates SW_TAGS and HW_TAGS > to use the unified kasan_shadow_enable() function. > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 > Signed-off-by: Sabyrzhan Tasbolatov > --- > Changes in v3: > - Only architectures that need deferred KASAN get runtime overhead > - Added kasan_shadow_initialized() for shadow memory readiness tracking > - kasan_enabled() now provides compile-time check for KASAN configuration > --- > include/linux/kasan-enabled.h | 34 ++ > include/linux/kasan.h | 6 ++ > mm/kasan/common.c | 9 + > mm/kasan/generic.c| 11 +++ > mm/kasan/hw_tags.c| 9 + > mm/kasan/sw_tags.c| 2 ++ > 6 files changed, 55 insertions(+), 16 deletions(-) > > diff --git a/include/linux/kasan-enabled.h b/include/linux/kasan-enabled.h > index 6f612d69ea0..fa99dc58f95 100644 > --- a/include/linux/kasan-enabled.h > +++ b/include/linux/kasan-enabled.h > @@ -4,32 +4,50 @@ > > #include > > -#ifdef CONFIG_KASAN_HW_TAGS > +/* Controls whether KASAN is enabled at all (compile-time check). */ > +static __always_inline bool kasan_enabled(void) > +{ > + return IS_ENABLED(CONFIG_KASAN); > +} > > +#ifdef CONFIG_ARCH_DEFER_KASAN > +/* > + * Global runtime flag for architectures that need deferred KASAN. > + * Switched to 'true' by the appropriate kasan_init_*() > + * once KASAN is fully initialized. > + */ > DECLARE_STATIC_KEY_FALSE(kasan_flag_enabled); > > -static __always_inline bool kasan_enabled(void) > +static __always_inline bool kasan_shadow_initialized(void) > { > return static_branch_likely(&kasan_flag_enabled); > } > > -static inline bool kasan_hw_tags_enabled(void) > +static inline void kasan_enable(void) > +{ > + static_branch_enable(&kasan_flag_enabled); > +} > +#else > +/* For architectures that can enable KASAN early, use compile-time check. */ > +static __always_inline bool kasan_shadow_initialized(void) > { > return kasan_enabled(); > } > > -#else /* CONFIG_KASAN_HW_TAGS */ > +/* No-op for architectures that don't need deferred KASAN. */ > +static inline void kasan_enable(void) {} > +#endif /* CONFIG_ARCH_DEFER_KASAN */ > > -static inline bool kasan_enabled(void) > +#ifdef CONFIG_KASAN_HW_TAGS > +static inline bool kasan_hw_tags_enabled(void) > { > - return IS_ENABLED(CONFIG_KASAN); > + return kasan_enabled(); > } > - > +#else > static inline bool kasan_hw_tags_enabled(void) > { > return false; > } > - > #endif /* CONFIG_KASAN_HW_TAGS */ > > #endif /* LINUX_KASAN_ENABLED_H */ > diff --git a/include/linux/kasan.h b/include/linux/kasan.h > index 890011071f2..51a8293d1af 100644 > --- a/include/linux/kasan.h > +++ b/include/linux/kasan.h > @@ -543,6 +543,12 @@ void kasan_report_async(void); > > #endif /* CONFIG_KASAN_HW_TAGS */ > > +#ifdef CONFIG_KASAN_GENERIC > +void __init kasan_init_generic(void); > +#else > +static inline void kasan_init_generic(void) { } > +#endif > + > #ifdef CONFIG_KASAN_SW_TAGS > void __init kasan_init_sw_tags(void); > #else > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > index ed4873e18c7..c3a6446404d 100644 > --- a/mm/kasan/common.c > +++ b/mm/kasan/common.c > @@ -32,6 +32,15 @@ > #include "kasan.h" > #include "../slab.h" > > +#ifdef CONFIG_ARCH_DEFER_KASAN > +/* > + * Definition of the unified static key declared in kasan-enabled.h. > + * This provides consistent runtime enable/disable across KASAN modes. > + */ > +DEFINE_STATIC_KEY_FALSE(kasan_flag_enabled); > +EXPORT_SYMBOL(kasan_flag_enabled); > +#endif > + > struct slab *kasan_addr_to_slab(const void *addr) > { > if (virt_addr_valid(addr)) > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index d54e89f8c3e..03b6d322ff6 100644 > ---
Re: [PATCH v3 07/12] kasan/loongarch: select ARCH_DEFER_KASAN and call kasan_init_generic
On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: > diff --git a/arch/loongarch/include/asm/kasan.h > b/arch/loongarch/include/asm/kasan.h > index 62f139a9c87..0e50e5b5e05 100644 > --- a/arch/loongarch/include/asm/kasan.h > +++ b/arch/loongarch/include/asm/kasan.h > @@ -66,7 +66,6 @@ > #define XKPRANGE_WC_SHADOW_OFFSET(KASAN_SHADOW_START + > XKPRANGE_WC_KASAN_OFFSET) > #define XKVRANGE_VC_SHADOW_OFFSET(KASAN_SHADOW_START + > XKVRANGE_VC_KASAN_OFFSET) > > -extern bool kasan_early_stage; > extern unsigned char kasan_early_shadow_page[PAGE_SIZE]; > > #define kasan_mem_to_shadow kasan_mem_to_shadow > @@ -75,12 +74,6 @@ void *kasan_mem_to_shadow(const void *addr); > #define kasan_shadow_to_mem kasan_shadow_to_mem > const void *kasan_shadow_to_mem(const void *shadow_addr); > > -#define kasan_arch_is_ready kasan_arch_is_ready > -static __always_inline bool kasan_arch_is_ready(void) > -{ > - return !kasan_early_stage; > -} > - > #define addr_has_metadata addr_has_metadata > static __always_inline bool addr_has_metadata(const void *addr) > { > diff --git a/arch/loongarch/mm/kasan_init.c b/arch/loongarch/mm/kasan_init.c > index d2681272d8f..cf8315f9119 100644 > --- a/arch/loongarch/mm/kasan_init.c > +++ b/arch/loongarch/mm/kasan_init.c > @@ -40,11 +40,9 @@ static pgd_t kasan_pg_dir[PTRS_PER_PGD] __initdata > __aligned(PAGE_SIZE); > #define __pte_none(early, pte) (early ? pte_none(pte) : \ > ((pte_val(pte) & _PFN_MASK) == (unsigned long)__pa(kasan_early_shadow_page))) > > -bool kasan_early_stage = true; > - > void *kasan_mem_to_shadow(const void *addr) > { > - if (!kasan_arch_is_ready()) { > + if (!kasan_enabled()) { This doesn't make sense, !kasan_enabled() is compile-time check which is always false here. > return (void *)(kasan_early_shadow_page); > } else { > unsigned long maddr = (unsigned long)addr;
Re: [PATCH v3 00/12] kasan: unify kasan_arch_is_ready() and remove arch-specific implementations
On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: > === Testing with patches > > Testing in v3: > > - Compiled every affected arch with no errors: > > $ make CC=clang LD=ld.lld AR=llvm-ar NM=llvm-nm STRIP=llvm-strip \ > OBJCOPY=llvm-objcopy OBJDUMP=llvm-objdump READELF=llvm-readelf \ > HOSTCC=clang HOSTCXX=clang++ HOSTAR=llvm-ar HOSTLD=ld.lld \ > ARCH=$ARCH > > $ clang --version > ClangBuiltLinux clang version 19.1.4 > Target: x86_64-unknown-linux-gnu > Thread model: posix > > - make ARCH=um produces the warning during compiling: > MODPOST Module.symvers > WARNING: modpost: vmlinux: section mismatch in reference: \ > kasan_init+0x43 (section: .ltext) -> \ > kasan_init_generic (section: .init.text) > > AFAIU, it's due to the code in arch/um/kernel/mem.c, where kasan_init() > is placed in own section ".kasan_init", which calls kasan_init_generic() > which is marked with "__init". > > - Booting via qemu-system- and running KUnit tests: > > * arm64 (GENERIC, HW_TAGS, SW_TAGS): no regression, same above results. > * x86_64 (GENERIC): no regression, no errors > It would be interesting to see whether ARCH_DEFER_KASAN=y arches work. These series add static key into __asan_load*()/_store*() which are called from everywhere, including the code patching static branches during the switch. I have suspicion that the code patching static branches during static key switch might not be prepared to the fact the current CPU might try to execute this static branch in the middle of switch.
Re: [PATCH v3 08/12] kasan/um: select ARCH_DEFER_KASAN and call kasan_init_generic
On 7/17/25 4:27 PM, Sabyrzhan Tasbolatov wrote: > UserMode Linux needs deferred KASAN initialization as it has a custom > kasan_arch_is_ready() implementation that tracks shadow memory readiness > via the kasan_um_is_ready flag. > > Select ARCH_DEFER_KASAN to enable the unified static key mechanism > for runtime KASAN control. Call kasan_init_generic() which handles > Generic KASAN initialization and enables the static key. > > Delete the key kasan_um_is_ready in favor of the unified kasan_enabled() > interface. > > Note that kasan_init_generic has __init macro, which is called by > kasan_init() which is not marked with __init in arch/um code. > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 > Signed-off-by: Sabyrzhan Tasbolatov > --- > Changes in v3: > - Added CONFIG_ARCH_DEFER_KASAN selection for proper runtime control > --- > arch/um/Kconfig | 1 + > arch/um/include/asm/kasan.h | 5 - > arch/um/kernel/mem.c| 4 ++-- > 3 files changed, 3 insertions(+), 7 deletions(-) > > diff --git a/arch/um/Kconfig b/arch/um/Kconfig > index f08e8a7fac9..fd6d78bba52 100644 > --- a/arch/um/Kconfig > +++ b/arch/um/Kconfig > @@ -8,6 +8,7 @@ config UML > select ARCH_WANTS_DYNAMIC_TASK_STRUCT > select ARCH_HAS_CPU_FINALIZE_INIT > select ARCH_HAS_FORTIFY_SOURCE > + select ARCH_DEFER_KASAN > select ARCH_HAS_GCOV_PROFILE_ALL > select ARCH_HAS_KCOV > select ARCH_HAS_STRNCPY_FROM_USER > diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h > index f97bb1f7b85..81bcdc0f962 100644 > --- a/arch/um/include/asm/kasan.h > +++ b/arch/um/include/asm/kasan.h > @@ -24,11 +24,6 @@ > > #ifdef CONFIG_KASAN > void kasan_init(void); > -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 */ > diff --git a/arch/um/kernel/mem.c b/arch/um/kernel/mem.c > index 76bec7de81b..058cb70e330 100644 > --- a/arch/um/kernel/mem.c > +++ b/arch/um/kernel/mem.c > @@ -21,9 +21,9 @@ > #include > #include > #include > +#include > > #ifdef CONFIG_KASAN > -int kasan_um_is_ready; > void kasan_init(void) > { > /* > @@ -32,7 +32,7 @@ void kasan_init(void) >*/ > kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > init_task.kasan_depth = 0; > - kasan_um_is_ready = true; > + kasan_init_generic(); I think this runs before jump_label_init(), and static keys shouldn't be switched before that.> } > > static void (*kasan_init_ptr)(void)
Re: [PATCH v4 5/9] kasan/loongarch: select ARCH_DEFER_KASAN and call kasan_init_generic
On 8/5/25 4:26 PM, Sabyrzhan Tasbolatov wrote: > LoongArch needs deferred KASAN initialization as it has a custom > kasan_arch_is_ready() implementation that tracks shadow memory > readiness via the kasan_early_stage flag. > > Select ARCH_DEFER_KASAN to enable the unified static key mechanism > for runtime KASAN control. Call kasan_init_generic() which handles > Generic KASAN initialization and enables the static key. > > Replace kasan_arch_is_ready() with kasan_enabled() and delete the > flag kasan_early_stage in favor of the unified kasan_enabled() > interface. > > Note that init_task.kasan_depth = 0 is called after kasan_init_generic(), > which is different than in other arch kasan_init(). This is left > unchanged as it cannot be tested. > > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217049 > Signed-off-by: Sabyrzhan Tasbolatov > --- > Changes in v4: > - Replaced !kasan_enabled() with !kasan_shadow_initialized() in > loongarch which selects ARCH_DEFER_KASAN (Andrey Ryabinin) > --- > arch/loongarch/Kconfig | 1 + > arch/loongarch/include/asm/kasan.h | 7 --- > arch/loongarch/mm/kasan_init.c | 8 ++-- > 3 files changed, 3 insertions(+), 13 deletions(-) > > diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig > index f0abc38c40a..f6304c073ec 100644 > --- a/arch/loongarch/Kconfig > +++ b/arch/loongarch/Kconfig > @@ -9,6 +9,7 @@ config LOONGARCH > select ACPI_PPTT if ACPI > select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI > select ARCH_BINFMT_ELF_STATE > + select ARCH_DEFER_KASAN > select ARCH_DISABLE_KASAN_INLINE > select ARCH_ENABLE_MEMORY_HOTPLUG > select ARCH_ENABLE_MEMORY_HOTREMOVE > diff --git a/arch/loongarch/include/asm/kasan.h > b/arch/loongarch/include/asm/kasan.h > index 62f139a9c87..0e50e5b5e05 100644 > --- a/arch/loongarch/include/asm/kasan.h > +++ b/arch/loongarch/include/asm/kasan.h > @@ -66,7 +66,6 @@ > #define XKPRANGE_WC_SHADOW_OFFSET(KASAN_SHADOW_START + > XKPRANGE_WC_KASAN_OFFSET) > #define XKVRANGE_VC_SHADOW_OFFSET(KASAN_SHADOW_START + > XKVRANGE_VC_KASAN_OFFSET) > > -extern bool kasan_early_stage; > extern unsigned char kasan_early_shadow_page[PAGE_SIZE]; > > #define kasan_mem_to_shadow kasan_mem_to_shadow > @@ -75,12 +74,6 @@ void *kasan_mem_to_shadow(const void *addr); > #define kasan_shadow_to_mem kasan_shadow_to_mem > const void *kasan_shadow_to_mem(const void *shadow_addr); > > -#define kasan_arch_is_ready kasan_arch_is_ready > -static __always_inline bool kasan_arch_is_ready(void) > -{ > - return !kasan_early_stage; > -} > - > #define addr_has_metadata addr_has_metadata > static __always_inline bool addr_has_metadata(const void *addr) > { > diff --git a/arch/loongarch/mm/kasan_init.c b/arch/loongarch/mm/kasan_init.c > index d2681272d8f..57fb6e98376 100644 > --- a/arch/loongarch/mm/kasan_init.c > +++ b/arch/loongarch/mm/kasan_init.c > @@ -40,11 +40,9 @@ static pgd_t kasan_pg_dir[PTRS_PER_PGD] __initdata > __aligned(PAGE_SIZE); > #define __pte_none(early, pte) (early ? pte_none(pte) : \ > ((pte_val(pte) & _PFN_MASK) == (unsigned long)__pa(kasan_early_shadow_page))) > > -bool kasan_early_stage = true; > - > void *kasan_mem_to_shadow(const void *addr) > { > - if (!kasan_arch_is_ready()) { > + if (!kasan_shadow_initialized()) { > return (void *)(kasan_early_shadow_page); > } else { > unsigned long maddr = (unsigned long)addr; > @@ -298,8 +296,6 @@ void __init kasan_init(void) > kasan_populate_early_shadow(kasan_mem_to_shadow((void *)VMALLOC_START), > kasan_mem_to_shadow((void > *)KFENCE_AREA_END)); > > - kasan_early_stage = false; > - There is a reason for this line to be here. Your patch will change the result of the follow up kasan_mem_to_shadow() call and feed the wrong address to kasan_map_populate() > /* Populate the linear mapping */ > for_each_mem_range(i, &pa_start, &pa_end) { > void *start = (void *)phys_to_virt(pa_start); > @@ -329,5 +325,5 @@ void __init kasan_init(void) > > /* At this point kasan is fully initialized. Enable error messages */ > init_task.kasan_depth = 0; > - pr_info("KernelAddressSanitizer initialized.\n"); > + kasan_init_generic(); > }
Re: [PATCH v4 6/9] kasan/um: select ARCH_DEFER_KASAN and call kasan_init_generic
On 8/5/25 4:26 PM, Sabyrzhan Tasbolatov wrote: > > diff --git a/arch/um/Kconfig b/arch/um/Kconfig > index 9083bfdb773..8d14c8fc2cd 100644 > --- a/arch/um/Kconfig > +++ b/arch/um/Kconfig > @@ -5,6 +5,7 @@ menu "UML-specific options" > config UML > bool > default y > + select ARCH_DEFER_KASAN select ARCH_DEFER_KASAN if STATIC_LINK > select ARCH_WANTS_DYNAMIC_TASK_STRUCT > select ARCH_HAS_CACHE_LINE_SIZE > select ARCH_HAS_CPU_FINALIZE_INIT > diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h > index f97bb1f7b85..81bcdc0f962 100644 > --- a/arch/um/include/asm/kasan.h > +++ b/arch/um/include/asm/kasan.h > @@ -24,11 +24,6 @@ > > #ifdef CONFIG_KASAN > void kasan_init(void); > -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 */