Messages in this thread | | | From | Patricia Alfonso <> | Date | Wed, 12 Feb 2020 16:37:59 -0800 | Subject | Re: [RFC PATCH v2] UML: add support for KASAN under x86_64 |
| |
On Tue, Feb 11, 2020 at 12:21 AM Johannes Berg <johannes@sipsolutions.net> wrote: > > Hi, > > Looks very nice! Some questions/comments below: > > > Depends on Constructor support in UML and is based off of > > "[RFC PATCH] um: implement CONFIG_CONSTRUCTORS for modules" > > (https://patchwork.ozlabs.org/patch/1234551/) > > I guess I should resend this as a proper patch then. Did you test > modules? I can try (later) too. > I have not tested modules - you might want to test modules before sending it at a proper patch. I just know that it works for the purposes of this KASAN project.
> > 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. > > That also means if I have say 512MB memory allocated for UML, KASAN will > use an *additional* 64, unlike on a "real" system, where KASAN will take > about 1/8th of the available physical memory, right? > Currently, the amount of shadow memory allocated is a constant based on the amount of user space address space in x86_64 since this is the host architecture I have focused on.
> > + help > > + This is the offset at which the ~2.25TB of shadow memory is > > + initialized > > Maybe that should say "mapped" instead of "initialized", since there are > relatively few machines on which it could actually all all be used? > Valid point!
> > +// used in kasan_mem_to_shadow to divide by 8 > > +#define KASAN_SHADOW_SCALE_SHIFT 3 > > nit: use /* */ style comments > Will do
> > +#define KASAN_SHADOW_START (KASAN_SHADOW_OFFSET) > > +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) > > + > > +#ifdef CONFIG_KASAN > > +void kasan_init(void); > > +#else > > +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..875e1827588b 100644 > > --- a/arch/um/kernel/Makefile > > +++ b/arch/um/kernel/Makefile > > @@ -8,6 +8,28 @@ > > # kernel. > > KCOV_INSTRUMENT := n > > > > +# The way UMl deals with the stack causes seemingly false positive KASAN > > +# reports such as: > > +# BUG: KASAN: stack-out-of-bounds in show_stack+0x15e/0x1fb > > +# Read of size 8 at addr 000000006184bbb0 by task swapper/1 > > +# ================================================================== > > +# BUG: KASAN: stack-out-of-bounds in dump_trace+0x141/0x1c5 > > +# Read of size 8 at addr 0000000071057eb8 by task swapper/1 > > +# ================================================================== > > +# BUG: KASAN: stack-out-of-bounds in get_wchan+0xd7/0x138 > > +# Read of size 8 at addr 0000000070e8fc80 by task systemd/1 > > +# > > +# With these files removed from instrumentation, those reports are > > +# eliminated, but KASAN still repeatedly reports a bug on syscall_stub_data: > > +# ================================================================== > > +# BUG: KASAN: stack-out-of-bounds in syscall_stub_data+0x299/0x2bf > > +# Read of size 128 at addr 0000000071457c50 by task swapper/1 > > So that's actually something to fix still? Just trying to understand, > I'll test it later. > Yes, I have not found a fix for these issues yet and even with these few files excluded from instrumentation, the syscall_stub_data error occurs(unless CONFIG_STACK is disabled, but CONFIG_STACK is enabled by default when using gcc to compile). It is unclear whether this is a bug that KASAN has found in UML or it is a mismatch of KASAN error detection on UML.
> > -extern int printf(const char *msg, ...); > > -static void early_print(void) > > +#ifdef CONFIG_KASAN > > +void kasan_init(void) > > { > > - printf("I'm super early, before constructors\n"); > > + kasan_map_memory((void *)KASAN_SHADOW_START, KASAN_SHADOW_SIZE); > > Heh, you *actually* based it on my patch, in git terms, not just in code > terms. I think you should just pick up the few lines that you need from > that patch and squash them into this one, I just posted that to > demonstrate more clearly what I meant :-) > I did base this on your patch. I figured it was more likely to get merged before this patch anyway. To clarify, do you want me to include your constructors patch with this one as a patchset?
> > +/** > > + * kasan_map_memory() - maps memory from @start with a size of @len. > > I think the () shouldn't be there? > Okay!
> > +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)); > > If that fails, can we even continue? > Probably not, but with this executing before main(), what is the best way to have an error occur? Or maybe there's a way we can just continue without KASAN enabled and print to the console that KASAN failed to initialize?
> johannes >
-- Patricia Alfonso
| |