| Message ID | 20200115182816.33892-1-trishalfonso@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [RFC] UML: add support for KASAN under x86_64 | expand |
Hi Patricia, On Wed, 2020-01-15 at 10:28 -0800, Patricia Alfonso wrote: > Make KASAN run on User Mode Linux on x86_64. Oh wow, awesome! Just what I always wanted :-) I tried this before and failed miserably ... mostly I thought we actually needed CONFIG_CONSTRUCTORS, which doesn't work (at least I hope my patch for it was reverted?) - do you know what's up with that? Couple questions, if you don't mind. > +#ifdef CONFIG_X86_64 > +#define KASAN_SHADOW_SIZE 0x100000000000UL > +#else > +#error "KASAN_SHADOW_SIZE is not defined in this sub-architecture" > +#endif Is it even possible today to compile ARCH=um on anything but x86_64? If yes, perhaps the above should be select HAVE_ARCH_KASAN if X86_64 or so? I assume KASAN itself has some dependencies though, but perhaps ARM 64-bit or POWERPC 64-bit could possibly run into this, if not X86 32-bit. > +++ b/arch/um/kernel/skas/Makefile > @@ -5,6 +5,12 @@ > > obj-y := clone.o mmu.o process.o syscall.o uaccess.o > > +ifdef CONFIG_UML > +# Do not instrument until after start_uml() because KASAN is not > +# initialized yet > +KASAN_SANITIZE := n > +endif Not sure I understand this, can anything in this file even get compiled without CONFIG_UML? > +++ b/kernel/Makefile > @@ -32,6 +32,12 @@ KCOV_INSTRUMENT_kcov.o := n > KASAN_SANITIZE_kcov.o := n > CFLAGS_kcov.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector) > > +ifdef CONFIG_UML > +# Do not istrument kasan on panic because it can be called before KASAN typo there - 'instrument' > +# is initialized > +KASAN_SANITIZE_panic.o := n > +endif but maybe UML shouldn't call panic() in such contexts, instead of this? I've had some major trouble with calling into the kernel before things are ready (or after we've started tearing things down), so that might be a good thing overall anyway? Could just do it this way and fix it later too though I guess. > +++ b/lib/Makefile > @@ -17,6 +17,16 @@ KCOV_INSTRUMENT_list_debug.o := n > KCOV_INSTRUMENT_debugobjects.o := n > KCOV_INSTRUMENT_dynamic_debug.o := n > > +# Don't sanatize typo > vsprintf or string functions in UM because they are used > +# before KASAN is initialized from cmdline parsing cmdline and kstrtox are > +# also called during uml initialization before KASAN is instrumented > +ifdef CONFIG_UML > +KASAN_SANITIZE_vsprintf.o := n > +KASAN_SANITIZE_string.o := n > +KASAN_SANITIZE_cmdline.o := n > +KASAN_SANITIZE_kstrtox.o := n > +endif I guess this can't be avoided. Very cool, I look forward to trying this out! :-) Thanks, johannes
On Wed, Jan 15, 2020 at 10:48 AM Johannes Berg <johannes@sipsolutions.net> wrote: > Couple questions, if you don't mind. > > > +#ifdef CONFIG_X86_64 > > +#define KASAN_SHADOW_SIZE 0x100000000000UL > > +#else > > +#error "KASAN_SHADOW_SIZE is not defined in this sub-architecture" > > +#endif > > Is it even possible today to compile ARCH=um on anything but x86_64? If > yes, perhaps the above should be > > select HAVE_ARCH_KASAN if X86_64 > > or so? I assume KASAN itself has some dependencies though, but perhaps > ARM 64-bit or POWERPC 64-bit could possibly run into this, if not X86 > 32-bit. > This seems like a good idea. I'll keep the #ifdef around KASAN_SHADOW_SIZE, but add "select HAVE_ARCH_KASAN if X86_64" as well. This will make extending it later easier. > > +++ b/arch/um/kernel/skas/Makefile > > @@ -5,6 +5,12 @@ > > > > obj-y := clone.o mmu.o process.o syscall.o uaccess.o > > > > +ifdef CONFIG_UML > > +# Do not instrument until after start_uml() because KASAN is not > > +# initialized yet > > +KASAN_SANITIZE := n > > +endif > > Not sure I understand this, can anything in this file even get compiled > without CONFIG_UML? > You are correct; this #ifdef was unnecessary. I will remove it. Thanks! > > +++ b/kernel/Makefile > > @@ -32,6 +32,12 @@ KCOV_INSTRUMENT_kcov.o := n > > KASAN_SANITIZE_kcov.o := n > > CFLAGS_kcov.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector) > > > > +ifdef CONFIG_UML > > +# Do not istrument kasan on panic because it can be called before KASAN > > typo there - 'instrument' > Thanks for catching that! > > +++ b/lib/Makefile > > @@ -17,6 +17,16 @@ KCOV_INSTRUMENT_list_debug.o := n > > KCOV_INSTRUMENT_debugobjects.o := n > > KCOV_INSTRUMENT_dynamic_debug.o := n > > > > +# Don't sanatize > > typo > Thanks for catching this, too! > Very cool, I look forward to trying this out! :-) > > Thanks, > johannes > Thank you so much for the comments! Best, Patricia -- Patricia Alfonso Software Engineer trishalfonso@google.com
Hi, > This seems like a good idea. I'll keep the #ifdef around > KASAN_SHADOW_SIZE, but add "select HAVE_ARCH_KASAN if X86_64" as well. > This will make extending it later easier. Yeah, that makes a lot of sense. I think once somebody (Anton? Richard?) start applying patches again, they will pick up my revert for CONFIG_CONSTRUCTORS: https://patchwork.ozlabs.org/patch/1204275/ (See there for why I had to revert it) If I remember correctly, KASAN depends on CONSTRUCTORS, so that revert will then break your patch here? And if I remember from looking at KASAN, some of the constructors there depended on initializing after the KASAN data structures were set up (or at least allocated)? It may be that you solved that by allocating the shadow so very early though. In any case, I think you should pick up that revert of CONFIG_CONSTRUCTORS and see what you have to do to make it still work, if that's possible. If not, then ... tricky, not sure what to do. Maybe then we could somehow hook in and have our own constructor that's called even before the compiler-emitted ASAN constructors, to allocate the necessary data structures. johannes
On Thu, 2020-01-16 at 08:57 +0100, Johannes Berg wrote: > > And if I remember from looking at KASAN, some of the constructors there > depended on initializing after the KASAN data structures were set up (or > at least allocated)? It may be that you solved that by allocating the > shadow so very early though. Actually, no ... it's still after main(), and the constructors run before. So I _think_ with the CONFIG_CONSTRUCTORS revert, this will no longer work (but happy to be proven wrong!), if so then I guess we do have to find a way to initialize the KASAN things from another (somehow earlier?) constructor ... Or find a way to fix CONFIG_CONSTRUCTORS and not revert, but I looked at it quite a bit and didn't. johannes
On Wed, Jan 15, 2020 at 11:56 PM Patricia Alfonso <trishalfonso@google.com> wrote: > > > +++ b/kernel/Makefile > > > @@ -32,6 +32,12 @@ KCOV_INSTRUMENT_kcov.o := n > > > KASAN_SANITIZE_kcov.o := n > > > CFLAGS_kcov.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector) > > > > > > +ifdef CONFIG_UML > > > +# Do not istrument kasan on panic because it can be called before KASAN > > > > typo there - 'instrument' > > > > Thanks for catching that! Hi Patricia, Very cool indeed! And will be a kunit killer feature! I can't parse this sentence (even with fixed), what is "kasan on panic"? Did you want to say "Do not instrument panic because it can be called before KASAN is initialized"? Or "Do not KASAN-instrument panic because it can be called before KASAN is initialized"? Though, "KASAN-instrument" looks somewhat redundant in this context.
On Wed, Jan 15, 2020 at 7:28 PM Patricia Alfonso <trishalfonso@google.com> wrote: > +config KASAN_SHADOW_OFFSET > + hex > + depends on KASAN > + default 0x100000000000 > + help > + This is the offset at which the ~2.25TB of shadow memory is > + initialized and used by KASAN for memory debugging. The default > + is 0x100000000000. What are restrictions on this value? In user-space we use 0x7fff8000 as a base (just below 2GB) and it's extremely profitable wrt codegen since it fits into immediate of most instructions. We can load and add the base with a short instruction: 2d8c: 48 81 c2 00 80 ff 7f add $0x7fff8000,%rdx Or even add base, load shadow and check it with a single 7-byte instruction: 1e4: 80 b8 00 80 ff 7f 00 cmpb $0x0,0x7fff8000(%rax) While with the large base, it takes 10 bytes just to load the const into a register (current x86 KASAN codegen): ffffffff81001571: 48 b8 00 00 00 00 00 fc ff df movabs $0xdffffc0000000000,%rax Most instructions don't have 8-byte immediates, so then we separately need to add/load/check.
On Wed, Jan 15, 2020 at 7:28 PM Patricia Alfonso <trishalfonso@google.com> wrote: > > Make KASAN run on User Mode Linux on x86_64. > diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile > index 5aa882011e04..f783a7dd863c 100644 > --- a/arch/um/kernel/Makefile > +++ b/arch/um/kernel/Makefile > @@ -8,6 +8,9 @@ > # kernel. > KCOV_INSTRUMENT := n > > +# Do not instrument on main.o It's always good to explain why. Otherwise it will be stuck there forever because nobody really knows why and if they will break something by removing it. This comment is also somewhat confusing. We don't instrument the whole dir, not just main.c? Should we ignore just this single file as comment says? > +KASAN_SANITIZE := n > +
> +void kasan_init(void) > +{ > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > + > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > + > + // unpoison the vmalloc region, which is start_vm -> end_vm > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > + > + init_task.kasan_depth = 0; > + pr_info("KernelAddressSanitizer initialized\n"); > +} Was this tested with stack instrumentation? Stack instrumentation changes what shadow is being read/written and when. We don't need to get it working right now, but if it does not work it would be nice to restrict the setting and leave some comment traces for future generations.
On Wed, Jan 15, 2020 at 7:28 PM Patricia Alfonso <trishalfonso@google.com> wrote: > diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h > new file mode 100644 > index 000000000000..ca4c43a35d41 > --- /dev/null > +++ b/arch/um/include/asm/kasan.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __ASM_UM_KASAN_H > +#define __ASM_UM_KASAN_H > + > +#include <linux/init.h> > +#include <linux/const.h> > + > +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL) > +#ifdef CONFIG_X86_64 > +#define KASAN_SHADOW_SIZE 0x100000000000UL How was this number computed? Can we replace this with some formula? I suspect this may be an order of magnitude off. Isn't 0x10000000000 enough? > +#else > +#error "KASAN_SHADOW_SIZE is not defined in this sub-architecture" > +#endif > + > +// used in kasan_mem_to_shadow to divide by 8 > +#define KASAN_SHADOW_SCALE_SHIFT 3 > + > +#define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET) > +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) > + > +#ifdef CONFIG_KASAN > +void kasan_init(void); > +void kasan_map_shadow(void); > +#else > +static inline void kasan_early_init(void) { } > +static inline void kasan_init(void) { } > +#endif /* CONFIG_KASAN */ > + > +void kasan_map_memory(void *start, unsigned long len); This better be moved under #ifdef CONFIG_KASAN, it's not defined otherwise, right? > +void kasan_unpoison_shadow(const void *address, size_t size); This is defined by <linux/kasan.h>. It's better to include that file where you need this function. Or there are some issues with that? > + > +#endif /* __ASM_UM_KASAN_H */ > diff --git a/arch/um/kernel/kasan_init_um.c b/arch/um/kernel/kasan_init_um.c > new file mode 100644 > index 000000000000..2e9a85216fb5 > --- /dev/null > +++ b/arch/um/kernel/kasan_init_um.c > @@ -0,0 +1,20 @@ > +// SPDX-License-Identifier: GPL-2.0 > +#include <asm/kasan.h> > +#include <linux/sched.h> > +#include <linux/sched/task.h> > +#include <asm/dma.h> > +#include <as-layout.h> > + > +void kasan_init(void) > +{ > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > + > + // unpoison the kernel text which is form uml_physmem -> uml_reserved Why do we need to unpoison _text_? Who is accessing shadow for it? Do you mean data/bss? But on a more general point, we just allocated it with mmap, mmap always gives zeroed memory and asan shadow is specifically arranged so that 0's mean "good". So I don't think we need to unpoison anything separately. What may be more useful is to poison (or better mprotect, unmap, not mmap) regions that kernel is not supposed to ever touch. One such region is shadow self-mapping (shadow for shadow), in user-space we mprotect that region. For KASAN we don't map shadow for user-space part of VM, but I don't know if UML has such separation. We could also protect other UML-specific regions if there are any, e.g does anybody read/write text? > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > + > + // unpoison the vmalloc region, which is start_vm -> end_vm > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > + > + init_task.kasan_depth = 0; > + pr_info("KernelAddressSanitizer initialized\n"); > +}
On Thu, Jan 16, 2020 at 9:03 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Thu, 2020-01-16 at 08:57 +0100, Johannes Berg wrote: > > > > And if I remember from looking at KASAN, some of the constructors there > > depended on initializing after the KASAN data structures were set up (or > > at least allocated)? It may be that you solved that by allocating the > > shadow so very early though. > > Actually, no ... it's still after main(), and the constructors run > before. > > So I _think_ with the CONFIG_CONSTRUCTORS revert, this will no longer > work (but happy to be proven wrong!), if so then I guess we do have to > find a way to initialize the KASAN things from another (somehow > earlier?) constructor ... > > Or find a way to fix CONFIG_CONSTRUCTORS and not revert, but I looked at > it quite a bit and didn't. Looking at this problem and at the number of KASAN_SANITIZE := n in Makefiles (some of which are pretty sad, e.g. ignoring string.c, kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we initialize KASAN too late. I think we need to do roughly what we do in user-space asan (because it is user-space asan!). Constructors run before main and it's really good, we need to initialize KASAN from these constructors. Or if that's not enough in all cases, also add own constructor/.preinit array entry to initialize as early as possible. All we need to do is to call mmap syscall, there is really no dependencies on anything kernel-related. This should resolve the problem with constructors (after they initialize KASAN, they can proceed to do anything they need) and it should get rid of most KASAN_SANITIZE (in particular, all of lib/Makefile and kernel/Makefile) and should fix stack instrumentation (in case it does not work now). The only tiny bit we should not instrument is the path from constructor up to mmap call.
On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > Looking at this problem and at the number of KASAN_SANITIZE := n in > Makefiles (some of which are pretty sad, e.g. ignoring string.c, > kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we > initialize KASAN too late. I think we need to do roughly what we do in > user-space asan (because it is user-space asan!). Constructors run > before main and it's really good, we need to initialize KASAN from > these constructors. Or if that's not enough in all cases, also add own > constructor/.preinit array entry to initialize as early as possible. We even control the linker in this case, so we can put something into the .preinit array *first*. > All we need to do is to call mmap syscall, there is really no > dependencies on anything kernel-related. OK. I wasn't really familiar with those details. > This should resolve the problem with constructors (after they > initialize KASAN, they can proceed to do anything they need) and it > should get rid of most KASAN_SANITIZE (in particular, all of > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > (in case it does not work now). The only tiny bit we should not > instrument is the path from constructor up to mmap call. That'd be great :) johannes
On Thu, Jan 16, 2020 at 10:20 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > > > Looking at this problem and at the number of KASAN_SANITIZE := n in > > Makefiles (some of which are pretty sad, e.g. ignoring string.c, > > kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we > > initialize KASAN too late. I think we need to do roughly what we do in > > user-space asan (because it is user-space asan!). Constructors run > > before main and it's really good, we need to initialize KASAN from > > these constructors. Or if that's not enough in all cases, also add own > > constructor/.preinit array entry to initialize as early as possible. > > We even control the linker in this case, so we can put something into > the .preinit array *first*. Even better! If we can reliably put something before constructors, we don't even need lazy init in constructors. > > All we need to do is to call mmap syscall, there is really no > > dependencies on anything kernel-related. > > OK. I wasn't really familiar with those details. > > > This should resolve the problem with constructors (after they > > initialize KASAN, they can proceed to do anything they need) and it > > should get rid of most KASAN_SANITIZE (in particular, all of > > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > > (in case it does not work now). The only tiny bit we should not > > instrument is the path from constructor up to mmap call. > > That'd be great :) > > johannes > > -- > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/2092169e6dd1f8d15f1db4b3787cc9fe596097b7.camel%40sipsolutions.net.
On Thu, Jan 16, 2020 at 10:39 PM Patricia Alfonso <trishalfonso@google.com> wrote: > > On Thu, Jan 16, 2020 at 1:23 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Thu, Jan 16, 2020 at 10:20 AM Johannes Berg > > <johannes@sipsolutions.net> wrote: > > > > > > On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > > > > > > > Looking at this problem and at the number of KASAN_SANITIZE := n in > > > > Makefiles (some of which are pretty sad, e.g. ignoring string.c, > > > > kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we > > > > initialize KASAN too late. I think we need to do roughly what we do in > > > > user-space asan (because it is user-space asan!). Constructors run > > > > before main and it's really good, we need to initialize KASAN from > > > > these constructors. Or if that's not enough in all cases, also add own > > > > constructor/.preinit array entry to initialize as early as possible. > > > > > I am not too happy with the number of KASAN_SANITIZE := n's either. > This sounds like a good idea. Let me look into it; I am not familiar > with constructors or .preint array. > > > > We even control the linker in this case, so we can put something into > > > the .preinit array *first*. > > > > Even better! If we can reliably put something before constructors, we > > don't even need lazy init in constructors. > > > > > > All we need to do is to call mmap syscall, there is really no > > > > dependencies on anything kernel-related. > > > > > > OK. I wasn't really familiar with those details. > > > > > > > This should resolve the problem with constructors (after they > > > > initialize KASAN, they can proceed to do anything they need) and it > > > > should get rid of most KASAN_SANITIZE (in particular, all of > > > > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > > > > (in case it does not work now). The only tiny bit we should not > > > > instrument is the path from constructor up to mmap call. > > This sounds like a great solution. I am getting this KASAN report: > "BUG: KASAN: stack-out-of-bounds in syscall_stub_data+0x2a5/0x2c7", > which is probably because of this stack instrumentation problem you > point out. [reposting to the list] If that part of the code I mentioned is instrumented, manifestation would be different -- stack instrumentation will try to access shadow, shadow is not mapped yet, so it would crash on the shadow access. What you are seeing looks like, well, a kernel bug where it does a bad stack access. Maybe it's KASAN actually _working_? :)
On Fri, Jan 17, 2020 at 10:59 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Thu, Jan 16, 2020 at 10:39 PM Patricia Alfonso > <trishalfonso@google.com> wrote: > > > > On Thu, Jan 16, 2020 at 1:23 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > On Thu, Jan 16, 2020 at 10:20 AM Johannes Berg > > > <johannes@sipsolutions.net> wrote: > > > > > > > > On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > > > > > > > > > Looking at this problem and at the number of KASAN_SANITIZE := n in > > > > > Makefiles (some of which are pretty sad, e.g. ignoring string.c, > > > > > kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we > > > > > initialize KASAN too late. I think we need to do roughly what we do in > > > > > user-space asan (because it is user-space asan!). Constructors run > > > > > before main and it's really good, we need to initialize KASAN from > > > > > these constructors. Or if that's not enough in all cases, also add own > > > > > constructor/.preinit array entry to initialize as early as possible. > > > > > > > > I am not too happy with the number of KASAN_SANITIZE := n's either. > > This sounds like a good idea. Let me look into it; I am not familiar > > with constructors or .preint array. > > > > > > We even control the linker in this case, so we can put something into > > > > the .preinit array *first*. > > > > > > Even better! If we can reliably put something before constructors, we > > > don't even need lazy init in constructors. > > > > > > > > All we need to do is to call mmap syscall, there is really no > > > > > dependencies on anything kernel-related. > > > > > > > > OK. I wasn't really familiar with those details. > > > > > > > > > This should resolve the problem with constructors (after they > > > > > initialize KASAN, they can proceed to do anything they need) and it > > > > > should get rid of most KASAN_SANITIZE (in particular, all of > > > > > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > > > > > (in case it does not work now). The only tiny bit we should not > > > > > instrument is the path from constructor up to mmap call. > > > > This sounds like a great solution. I am getting this KASAN report: > > "BUG: KASAN: stack-out-of-bounds in syscall_stub_data+0x2a5/0x2c7", > > which is probably because of this stack instrumentation problem you > > point out. > > [reposting to the list] > > If that part of the code I mentioned is instrumented, manifestation > would be different -- stack instrumentation will try to access shadow, > shadow is not mapped yet, so it would crash on the shadow access. > > What you are seeing looks like, well, a kernel bug where it does a bad > stack access. Maybe it's KASAN actually _working_? :) Though, stack instrumentation may have issues with longjmp-like things. I would suggest first turning off stack instrumentation and getting that work. Solving problems one-by-one is always easier. If you need help debugging this, please post more info: patch, what you are doing, full kernel output (preferably from start, if it's not too lengthy).
On Fri, Jan 17, 2020 at 11:03 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Fri, Jan 17, 2020 at 10:59 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Thu, Jan 16, 2020 at 10:39 PM Patricia Alfonso > > <trishalfonso@google.com> wrote: > > > > > > On Thu, Jan 16, 2020 at 1:23 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > > > On Thu, Jan 16, 2020 at 10:20 AM Johannes Berg > > > > <johannes@sipsolutions.net> wrote: > > > > > > > > > > On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > > > > > > > > > > > Looking at this problem and at the number of KASAN_SANITIZE := n in > > > > > > Makefiles (some of which are pretty sad, e.g. ignoring string.c, > > > > > > kstrtox.c, vsprintf.c -- that's where the bugs are!), I think we > > > > > > initialize KASAN too late. I think we need to do roughly what we do in > > > > > > user-space asan (because it is user-space asan!). Constructors run > > > > > > before main and it's really good, we need to initialize KASAN from > > > > > > these constructors. Or if that's not enough in all cases, also add own > > > > > > constructor/.preinit array entry to initialize as early as possible. > > > > > > > > > > > I am not too happy with the number of KASAN_SANITIZE := n's either. > > > This sounds like a good idea. Let me look into it; I am not familiar > > > with constructors or .preint array. > > > > > > > > We even control the linker in this case, so we can put something into > > > > > the .preinit array *first*. > > > > > > > > Even better! If we can reliably put something before constructors, we > > > > don't even need lazy init in constructors. > > > > > > > > > > All we need to do is to call mmap syscall, there is really no > > > > > > dependencies on anything kernel-related. > > > > > > > > > > OK. I wasn't really familiar with those details. > > > > > > > > > > > This should resolve the problem with constructors (after they > > > > > > initialize KASAN, they can proceed to do anything they need) and it > > > > > > should get rid of most KASAN_SANITIZE (in particular, all of > > > > > > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > > > > > > (in case it does not work now). The only tiny bit we should not > > > > > > instrument is the path from constructor up to mmap call. > > > > > > This sounds like a great solution. I am getting this KASAN report: > > > "BUG: KASAN: stack-out-of-bounds in syscall_stub_data+0x2a5/0x2c7", > > > which is probably because of this stack instrumentation problem you > > > point out. > > > > [reposting to the list] > > > > If that part of the code I mentioned is instrumented, manifestation > > would be different -- stack instrumentation will try to access shadow, > > shadow is not mapped yet, so it would crash on the shadow access. > > > > What you are seeing looks like, well, a kernel bug where it does a bad > > stack access. Maybe it's KASAN actually _working_? :) > > Though, stack instrumentation may have issues with longjmp-like things. > I would suggest first turning off stack instrumentation and getting > that work. Solving problems one-by-one is always easier. > If you need help debugging this, please post more info: patch, what > you are doing, full kernel output (preferably from start, if it's not > too lengthy). I see syscall_stub_data does some weird things with stack (stack copy?). Maybe we just need to ignore accesses there: individual accesses, or whole function/file.
On Thu, Jan 16, 2020 at 12:03 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > On Thu, 2020-01-16 at 08:57 +0100, Johannes Berg wrote: > > > > And if I remember from looking at KASAN, some of the constructors there > > depended on initializing after the KASAN data structures were set up (or > > at least allocated)? It may be that you solved that by allocating the > > shadow so very early though. > > Actually, no ... it's still after main(), and the constructors run > before. > > So I _think_ with the CONFIG_CONSTRUCTORS revert, this will no longer > work (but happy to be proven wrong!), if so then I guess we do have to > find a way to initialize the KASAN things from another (somehow > earlier?) constructor ... > > Or find a way to fix CONFIG_CONSTRUCTORS and not revert, but I looked at > it quite a bit and didn't. > > johannes I've looked at this quite extensively over the past week or so. I was able to initialize KASAN as one of the first things that gets executed in main(), but constructors are, in fact, needed before main(). I think it might be best to reintroduce constructors in a limited way to allow KASAN to work in UML. I have done as much testing as I can on my machine and this limited version seems to work, except when STATIC_LINK is set. I will send some patches of what I have done so far and we can talk more about it there. I would like to add your name, Johannes, as a co-developed-by on that patch. If there is a better way to give you credit for this, please let me know.
On Fri, Jan 17, 2020 at 2:05 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Fri, Jan 17, 2020 at 11:03 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Fri, Jan 17, 2020 at 10:59 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > On Thu, Jan 16, 2020 at 10:39 PM Patricia Alfonso > > > <trishalfonso@google.com> wrote: > > > > > > > > On Thu, Jan 16, 2020 at 1:23 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > > > > > On Thu, Jan 16, 2020 at 10:20 AM Johannes Berg > > > > > <johannes@sipsolutions.net> wrote: > > > > > > > > > > > > On Thu, 2020-01-16 at 10:18 +0100, Dmitry Vyukov wrote: > > > > > > > > > > > > > > This should resolve the problem with constructors (after they > > > > > > > initialize KASAN, they can proceed to do anything they need) and it > > > > > > > should get rid of most KASAN_SANITIZE (in particular, all of > > > > > > > lib/Makefile and kernel/Makefile) and should fix stack instrumentation > > > > > > > (in case it does not work now). The only tiny bit we should not > > > > > > > instrument is the path from constructor up to mmap call. > > > > By initializing KASAN as the first thing that executes, I have been able to get rid of most of the "KASAN_SANITIZE := n" lines and I am very happy about that. Thanks for the suggestions! > > > If that part of the code I mentioned is instrumented, manifestation > > > would be different -- stack instrumentation will try to access shadow, > > > shadow is not mapped yet, so it would crash on the shadow access. > > > > > > What you are seeing looks like, well, a kernel bug where it does a bad > > > stack access. Maybe it's KASAN actually _working_? :) > > > > Though, stack instrumentation may have issues with longjmp-like things. > > I would suggest first turning off stack instrumentation and getting > > that work. Solving problems one-by-one is always easier. > > If you need help debugging this, please post more info: patch, what > > you are doing, full kernel output (preferably from start, if it's not > > too lengthy). > > I see syscall_stub_data does some weird things with stack (stack > copy?). Maybe we just need to ignore accesses there: individual > accesses, or whole function/file. It is still not clear whether the syscall_stub_data errors are false positives, but while moving the kasan_init() to be as early as possible in main(), I ran into a few more stack-related errors like this(show_stack, dump_trace, and get_wchan). I will be taking your advice to focus on one thing at a time and temporarily disable stack instrumentation wherever possible. -- Patricia Alfonso
Hi Patricia, > I've looked at this quite extensively over the past week or so. I was > able to initialize KASAN as one of the first things that gets executed > in main(), but constructors are, in fact, needed before main(). They're called before main, by the dynamic loader, or libc, or whatever magic is built into the binary, right? But what do you mean by "needed"? > I > think it might be best to reintroduce constructors in a limited way to > allow KASAN to work in UML. I guess I'd have to see that. > I have done as much testing as I can on my > machine and this limited version seems to work, except when > STATIC_LINK is set. I will send some patches of what I have done so > far and we can talk more about it there. I would like to add your > name, Johannes, as a co-developed-by on that patch. If there is a > better way to give you credit for this, please let me know. I think you give me way too much credit, but I'm not going to complain either way :-) I'll post in a minute what I had in mind. johannes
On Thu, Jan 16, 2020 at 12:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Wed, Jan 15, 2020 at 7:28 PM Patricia Alfonso > <trishalfonso@google.com> wrote: > > +config KASAN_SHADOW_OFFSET > > + hex > > + depends on KASAN > > + default 0x100000000000 > > + help > > + This is the offset at which the ~2.25TB of shadow memory is > > + initialized and used by KASAN for memory debugging. The default > > + is 0x100000000000. > > What are restrictions on this value? The only restriction is that there is enough space there to map all of the KASAN shadow memory without conflicting with anything else. > In user-space we use 0x7fff8000 as a base (just below 2GB) and it's > extremely profitable wrt codegen since it fits into immediate of most > instructions. > We can load and add the base with a short instruction: > 2d8c: 48 81 c2 00 80 ff 7f add $0x7fff8000,%rdx > Or even add base, load shadow and check it with a single 7-byte instruction: > 1e4: 80 b8 00 80 ff 7f 00 cmpb $0x0,0x7fff8000(%rax) > I just tested with 0x7fff8000 as the KASAN_SHADOW_OFFSET and it worked so I can make that the default if it will be more efficient.
On Thu, Jan 16, 2020 at 12:53 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > +void kasan_init(void) > > +{ > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > + > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > + > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > + > > + init_task.kasan_depth = 0; > > + pr_info("KernelAddressSanitizer initialized\n"); > > +} > > Was this tested with stack instrumentation? Stack instrumentation > changes what shadow is being read/written and when. We don't need to > get it working right now, but if it does not work it would be nice to > restrict the setting and leave some comment traces for future > generations. If you are referring to KASAN_STACK_ENABLE, I just tested it and it seems to work fine.
On Wed, Feb 12, 2020 at 12:48 AM Patricia Alfonso <trishalfonso@google.com> wrote: > > On Thu, Jan 16, 2020 at 12:44 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > On Wed, Jan 15, 2020 at 7:28 PM Patricia Alfonso > > <trishalfonso@google.com> wrote: > > > +config KASAN_SHADOW_OFFSET > > > + hex > > > + depends on KASAN > > > + default 0x100000000000 > > > + help > > > + This is the offset at which the ~2.25TB of shadow memory is > > > + initialized and used by KASAN for memory debugging. The default > > > + is 0x100000000000. > > > > What are restrictions on this value? > The only restriction is that there is enough space there to map all of > the KASAN shadow memory without conflicting with anything else. > > > In user-space we use 0x7fff8000 as a base (just below 2GB) and it's > > extremely profitable wrt codegen since it fits into immediate of most > > instructions. > > We can load and add the base with a short instruction: > > 2d8c: 48 81 c2 00 80 ff 7f add $0x7fff8000,%rdx > > Or even add base, load shadow and check it with a single 7-byte instruction: > > 1e4: 80 b8 00 80 ff 7f 00 cmpb $0x0,0x7fff8000(%rax) > > > I just tested with 0x7fff8000 as the KASAN_SHADOW_OFFSET and it worked > so I can make that the default if it will be more efficient. I think it's the right thing to do if it works.
On Wed, Feb 12, 2020 at 1:19 AM Patricia Alfonso <trishalfonso@google.com> wrote: > > On Thu, Jan 16, 2020 at 12:53 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > +void kasan_init(void) > > > +{ > > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > > + > > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > > + > > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > > + > > > + init_task.kasan_depth = 0; > > > + pr_info("KernelAddressSanitizer initialized\n"); > > > +} > > > > Was this tested with stack instrumentation? Stack instrumentation > > changes what shadow is being read/written and when. We don't need to > > get it working right now, but if it does not work it would be nice to > > restrict the setting and leave some comment traces for future > > generations. > If you are referring to KASAN_STACK_ENABLE, I just tested it and it > seems to work fine. I mean stack instrumentation which is enabled with CONFIG_KASAN_STACK.
On Tue, Feb 11, 2020 at 10:24 PM Dmitry Vyukov <dvyukov@google.com> wrote: > > On Wed, Feb 12, 2020 at 1:19 AM Patricia Alfonso > <trishalfonso@google.com> wrote: > > > > On Thu, Jan 16, 2020 at 12:53 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > > +void kasan_init(void) > > > > +{ > > > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > > > + > > > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > > > + > > > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > > > + > > > > + init_task.kasan_depth = 0; > > > > + pr_info("KernelAddressSanitizer initialized\n"); > > > > +} > > > > > > Was this tested with stack instrumentation? Stack instrumentation > > > changes what shadow is being read/written and when. We don't need to > > > get it working right now, but if it does not work it would be nice to > > > restrict the setting and leave some comment traces for future > > > generations. > > If you are referring to KASAN_STACK_ENABLE, I just tested it and it > > seems to work fine. > > > I mean stack instrumentation which is enabled with CONFIG_KASAN_STACK. I believe I was testing with CONFIG_KASAN_STACK set to 1 since that is the default value when compiling with GCC.The syscall_stub_data error disappears when the value of CONFIG_KASAN_STACK is 0, though.
On Wed, Feb 12, 2020 at 11:25 PM Patricia Alfonso <trishalfonso@google.com> wrote: > > On Wed, Feb 12, 2020 at 1:19 AM Patricia Alfonso > > <trishalfonso@google.com> wrote: > > > > > > On Thu, Jan 16, 2020 at 12:53 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > > > > +void kasan_init(void) > > > > > +{ > > > > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > > > > + > > > > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > > > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > > > > + > > > > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > > > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > > > > + > > > > > + init_task.kasan_depth = 0; > > > > > + pr_info("KernelAddressSanitizer initialized\n"); > > > > > +} > > > > > > > > Was this tested with stack instrumentation? Stack instrumentation > > > > changes what shadow is being read/written and when. We don't need to > > > > get it working right now, but if it does not work it would be nice to > > > > restrict the setting and leave some comment traces for future > > > > generations. > > > If you are referring to KASAN_STACK_ENABLE, I just tested it and it > > > seems to work fine. > > > > > > I mean stack instrumentation which is enabled with CONFIG_KASAN_STACK. > > I believe I was testing with CONFIG_KASAN_STACK set to 1 since that is > the default value when compiling with GCC.The syscall_stub_data error > disappears when the value of CONFIG_KASAN_STACK is 0, though. Then I would either disable it for now for UML, or try to unpoision stack or ignore accesses.
On Wed, Feb 12, 2020 at 9:40 PM 'Dmitry Vyukov' via kasan-dev <kasan-dev@googlegroups.com> wrote: > > On Wed, Feb 12, 2020 at 11:25 PM Patricia Alfonso > <trishalfonso@google.com> wrote: > > > On Wed, Feb 12, 2020 at 1:19 AM Patricia Alfonso > > > <trishalfonso@google.com> wrote: > > > > > > > > On Thu, Jan 16, 2020 at 12:53 AM Dmitry Vyukov <dvyukov@google.com> wrote: > > > > > > > > > > > +void kasan_init(void) > > > > > > +{ > > > > > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > > > > > + > > > > > > + // unpoison the kernel text which is form uml_physmem -> uml_reserved > > > > > > + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); > > > > > > + > > > > > > + // unpoison the vmalloc region, which is start_vm -> end_vm > > > > > > + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); > > > > > > + > > > > > > + init_task.kasan_depth = 0; > > > > > > + pr_info("KernelAddressSanitizer initialized\n"); > > > > > > +} > > > > > > > > > > Was this tested with stack instrumentation? Stack instrumentation > > > > > changes what shadow is being read/written and when. We don't need to > > > > > get it working right now, but if it does not work it would be nice to > > > > > restrict the setting and leave some comment traces for future > > > > > generations. > > > > If you are referring to KASAN_STACK_ENABLE, I just tested it and it > > > > seems to work fine. > > > > > > > > > I mean stack instrumentation which is enabled with CONFIG_KASAN_STACK. > > > > I believe I was testing with CONFIG_KASAN_STACK set to 1 since that is > > the default value when compiling with GCC.The syscall_stub_data error > > disappears when the value of CONFIG_KASAN_STACK is 0, though. > > > Then I would either disable it for now for UML, or try to unpoision > stack or ignore accesses. > Okay, I'll probably disable it in UML for now.
diff --git a/arch/um/Kconfig b/arch/um/Kconfig index 6f0edd0c0220..99c68863e7e9 100644 --- a/arch/um/Kconfig +++ b/arch/um/Kconfig @@ -8,6 +8,7 @@ config UML select ARCH_HAS_KCOV select ARCH_NO_PREEMPT select HAVE_ARCH_AUDITSYSCALL + select HAVE_ARCH_KASAN select HAVE_ARCH_SECCOMP_FILTER select HAVE_ASM_MODVERSIONS select HAVE_UID16 @@ -198,6 +199,15 @@ config UML_TIME_TRAVEL_SUPPORT It is safe to say Y, but you probably don't need this. +config KASAN_SHADOW_OFFSET + hex + depends on KASAN + default 0x100000000000 + help + This is the offset at which the ~2.25TB of shadow memory is + initialized and used by KASAN for memory debugging. The default + is 0x100000000000. + endmenu source "arch/um/drivers/Kconfig" diff --git a/arch/um/include/asm/dma.h b/arch/um/include/asm/dma.h index fdc53642c718..8aafd60d62bb 100644 --- a/arch/um/include/asm/dma.h +++ b/arch/um/include/asm/dma.h @@ -5,6 +5,7 @@ #include <asm/io.h> extern unsigned long uml_physmem; +extern unsigned long long physmem_size; #define MAX_DMA_ADDRESS (uml_physmem) diff --git a/arch/um/include/asm/kasan.h b/arch/um/include/asm/kasan.h new file mode 100644 index 000000000000..ca4c43a35d41 --- /dev/null +++ b/arch/um/include/asm/kasan.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ASM_UM_KASAN_H +#define __ASM_UM_KASAN_H + +#include <linux/init.h> +#include <linux/const.h> + +#define KASAN_SHADOW_OFFSET _AC(CONFIG_KASAN_SHADOW_OFFSET, UL) +#ifdef CONFIG_X86_64 +#define KASAN_SHADOW_SIZE 0x100000000000UL +#else +#error "KASAN_SHADOW_SIZE is not defined in this sub-architecture" +#endif + +// used in kasan_mem_to_shadow to divide by 8 +#define KASAN_SHADOW_SCALE_SHIFT 3 + +#define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET) +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) + +#ifdef CONFIG_KASAN +void kasan_init(void); +void kasan_map_shadow(void); +#else +static inline void kasan_early_init(void) { } +static inline void kasan_init(void) { } +#endif /* CONFIG_KASAN */ + +void kasan_map_memory(void *start, unsigned long len); +void kasan_unpoison_shadow(const void *address, size_t size); + +#endif /* __ASM_UM_KASAN_H */ diff --git a/arch/um/kernel/Makefile b/arch/um/kernel/Makefile index 5aa882011e04..f783a7dd863c 100644 --- a/arch/um/kernel/Makefile +++ b/arch/um/kernel/Makefile @@ -8,6 +8,9 @@ # kernel. KCOV_INSTRUMENT := n +# Do not instrument on main.o +KASAN_SANITIZE := n + CPPFLAGS_vmlinux.lds := -DSTART=$(LDS_START) \ -DELF_ARCH=$(LDS_ELF_ARCH) \ -DELF_FORMAT=$(LDS_ELF_FORMAT) \ @@ -24,6 +27,7 @@ obj-$(CONFIG_GPROF) += gprof_syms.o obj-$(CONFIG_GCOV) += gmon_syms.o obj-$(CONFIG_EARLY_PRINTK) += early_printk.o obj-$(CONFIG_STACKTRACE) += stacktrace.o +obj-$(CONFIG_KASAN) += kasan_init_um.o USER_OBJS := config.o diff --git a/arch/um/kernel/kasan_init_um.c b/arch/um/kernel/kasan_init_um.c new file mode 100644 index 000000000000..2e9a85216fb5 --- /dev/null +++ b/arch/um/kernel/kasan_init_um.c @@ -0,0 +1,20 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <asm/kasan.h> +#include <linux/sched.h> +#include <linux/sched/task.h> +#include <asm/dma.h> +#include <as-layout.h> + +void kasan_init(void) +{ + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); + + // unpoison the kernel text which is form uml_physmem -> uml_reserved + kasan_unpoison_shadow((void *)uml_physmem, physmem_size); + + // unpoison the vmalloc region, which is start_vm -> end_vm + kasan_unpoison_shadow((void *)start_vm, (end_vm - start_vm + 1)); + + init_task.kasan_depth = 0; + pr_info("KernelAddressSanitizer initialized\n"); +} diff --git a/arch/um/kernel/skas/Makefile b/arch/um/kernel/skas/Makefile index f3d494a4fd9b..d68f447274e5 100644 --- a/arch/um/kernel/skas/Makefile +++ b/arch/um/kernel/skas/Makefile @@ -5,6 +5,12 @@ obj-y := clone.o mmu.o process.o syscall.o uaccess.o +ifdef CONFIG_UML +# Do not instrument until after start_uml() because KASAN is not +# initialized yet +KASAN_SANITIZE := n +endif + # clone.o is in the stub, so it can't be built with profiling # GCC hardened also auto-enables -fpic, but we need %ebx so it can't work -> # disable it diff --git a/arch/um/kernel/um_arch.c b/arch/um/kernel/um_arch.c index 0f40eccbd759..73cd159d28e8 100644 --- a/arch/um/kernel/um_arch.c +++ b/arch/um/kernel/um_arch.c @@ -14,6 +14,7 @@ #include <linux/sched/task.h> #include <linux/kmsg_dump.h> +#include <asm/kasan.h> #include <asm/pgtable.h> #include <asm/processor.h> #include <asm/sections.h> @@ -227,6 +228,8 @@ static struct notifier_block panic_exit_notifier = { void uml_finishsetup(void) { + kasan_init(); + atomic_notifier_chain_register(&panic_notifier_list, &panic_exit_notifier); diff --git a/arch/um/os-Linux/mem.c b/arch/um/os-Linux/mem.c index 3c1b77474d2d..ef282bacc58e 100644 --- a/arch/um/os-Linux/mem.c +++ b/arch/um/os-Linux/mem.c @@ -17,6 +17,24 @@ #include <init.h> #include <os.h> +/** + * kasan_map_memory() - maps memory from @start with a size of @len + * @start: the start address of the memory to be mapped + * @len: the length of the memory to be mapped + * + * This function is used to map shadow memory for KASAN in uml + */ +void kasan_map_memory(void *start, size_t len) +{ + if (mmap(start, + len, + PROT_READ|PROT_WRITE, + MAP_FIXED|MAP_ANONYMOUS|MAP_PRIVATE|MAP_NORESERVE, + -1, + 0) == MAP_FAILED) + os_info("Couldn't allocate shadow memory %s", strerror(errno)); +} + /* Set by make_tempfile() during early boot. */ static char *tempdir = NULL; diff --git a/arch/um/os-Linux/user_syms.c b/arch/um/os-Linux/user_syms.c index 715594fe5719..cb667c9225ab 100644 --- a/arch/um/os-Linux/user_syms.c +++ b/arch/um/os-Linux/user_syms.c @@ -27,10 +27,10 @@ EXPORT_SYMBOL(strstr); #ifndef __x86_64__ extern void *memcpy(void *, const void *, size_t); EXPORT_SYMBOL(memcpy); -#endif - EXPORT_SYMBOL(memmove); EXPORT_SYMBOL(memset); +#endif + EXPORT_SYMBOL(printf); /* Here, instead, I can provide a fake prototype. Yes, someone cares: genksyms. diff --git a/arch/x86/um/Makefile b/arch/x86/um/Makefile index 33c51c064c77..7dbd76c546fe 100644 --- a/arch/x86/um/Makefile +++ b/arch/x86/um/Makefile @@ -26,7 +26,8 @@ else obj-y += syscalls_64.o vdso/ -subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o +subarch-y = ../lib/csum-partial_64.o ../lib/memcpy_64.o ../entry/thunk_64.o \ + ../lib/memmove_64.o ../lib/memset_64.o endif diff --git a/arch/x86/um/vdso/Makefile b/arch/x86/um/vdso/Makefile index 0caddd6acb22..450efa0fb694 100644 --- a/arch/x86/um/vdso/Makefile +++ b/arch/x86/um/vdso/Makefile @@ -3,6 +3,9 @@ # Building vDSO images for x86. # +# do not instrument on vdso because KASAN is not compatible with user mode +KASAN_SANITIZE := n + # Prevents link failures: __sanitizer_cov_trace_pc() is not linked in. KCOV_INSTRUMENT := n diff --git a/kernel/Makefile b/kernel/Makefile index f2cc0d118a0b..4fbb72cb253f 100644 --- a/kernel/Makefile +++ b/kernel/Makefile @@ -32,6 +32,12 @@ KCOV_INSTRUMENT_kcov.o := n KASAN_SANITIZE_kcov.o := n CFLAGS_kcov.o := $(call cc-option, -fno-conserve-stack -fno-stack-protector) +ifdef CONFIG_UML +# Do not istrument kasan on panic because it can be called before KASAN +# is initialized +KASAN_SANITIZE_panic.o := n +endif + # cond_syscall is currently not LTO compatible CFLAGS_sys_ni.o = $(DISABLE_LTO) diff --git a/lib/Makefile b/lib/Makefile index 93217d44237f..e28dc5b06ae2 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -17,6 +17,16 @@ KCOV_INSTRUMENT_list_debug.o := n KCOV_INSTRUMENT_debugobjects.o := n KCOV_INSTRUMENT_dynamic_debug.o := n +# Don't sanatize vsprintf or string functions in UM because they are used +# before KASAN is initialized from cmdline parsing cmdline and kstrtox are +# also called during uml initialization before KASAN is instrumented +ifdef CONFIG_UML +KASAN_SANITIZE_vsprintf.o := n +KASAN_SANITIZE_string.o := n +KASAN_SANITIZE_cmdline.o := n +KASAN_SANITIZE_kstrtox.o := n +endif + # Early boot use of cmdline, don't instrument it ifdef CONFIG_AMD_MEM_ENCRYPT KASAN_SANITIZE_string.o := n
Make KASAN run on User Mode Linux on x86_64. The location of the KASAN shadow memory, starting at KASAN_SHADOW_OFFSET, can be configured using the KASAN_SHADOW_OFFSET option. UML uses roughly 18TB of address space, and KASAN requires 1/8th of this. For this reason, the default location is 0x100000000000. There is usually enough free space at this location; however, it is a config option so that it can be easily changed if needed. Functions that are used before KASAN is initialized are excluded from instrumentation. The UML-specific KASAN initializer uses mmap to map the roughly 2.25TB of shadow memory to the location defined by KASAN_SHADOW_OFFSET and ensures that the address space used by the kernel text and the vmalloc region is not poisoned at initialization. Signed-off-by: Patricia Alfonso <trishalfonso@google.com> --- arch/um/Kconfig | 10 ++++++++++ arch/um/include/asm/dma.h | 1 + arch/um/include/asm/kasan.h | 32 ++++++++++++++++++++++++++++++++ arch/um/kernel/Makefile | 4 ++++ arch/um/kernel/kasan_init_um.c | 20 ++++++++++++++++++++ arch/um/kernel/skas/Makefile | 6 ++++++ arch/um/kernel/um_arch.c | 3 +++ arch/um/os-Linux/mem.c | 18 ++++++++++++++++++ arch/um/os-Linux/user_syms.c | 4 ++-- arch/x86/um/Makefile | 3 ++- arch/x86/um/vdso/Makefile | 3 +++ kernel/Makefile | 6 ++++++ lib/Makefile | 10 ++++++++++ 13 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 arch/um/include/asm/kasan.h create mode 100644 arch/um/kernel/kasan_init_um.c