From: Vincent Whitchurch <vincent.whitchurch@axis.com>
To: David Gow <davidgow@google.com>
Cc: Johannes Berg <johannes@sipsolutions.net>,
Patricia Alfonso <trishalfonso@google.com>,
Jeff Dike <jdike@addtoit.com>,
Richard Weinberger <richard@nod.at>,
Anton Ivanov <anton.ivanov@cambridgegreys.com>,
Andrey Ryabinin <aryabinin@virtuozzo.com>,
Dmitry Vyukov <dvyukov@google.com>,
Brendan Higgins <brendanhiggins@google.com>,
kasan-dev <kasan-dev@googlegroups.com>,
LKML <linux-kernel@vger.kernel.org>,
linux-um <linux-um@lists.infradead.org>,
Daniel Axtens <dja@axtens.net>
Subject: Re: [PATCH] UML: add support for KASAN under x86_64
Date: Wed, 25 May 2022 13:17:56 +0200 [thread overview]
Message-ID: <20220525111756.GA15955@axis.com> (raw)
In-Reply-To: <CABVgOSnTX_e+tzR6c3KnGhDidVtEoUdtt_CJ62g2+MQDMp657g@mail.gmail.com>
On Tue, May 24, 2022 at 09:35:33PM +0200, David Gow wrote:
> On Tue, May 24, 2022 at 3:34 AM Vincent Whitchurch
> <vincent.whitchurch@axis.com> wrote:
> > It works both with and without KASAN_VMALLOC. KASAN_STACK works too
> > after I disabled sanitization of the stacktrace code. All kasan kunit
> > tests pass and the test_kasan.ko module works too.
>
> I've got this running myself, and can confirm the kasan tests work
> under kunit_tool in most cases, though there are a couple of failures
> when built with clang/llvm:
> [11:56:30] # kasan_global_oob_right: EXPECTATION FAILED at lib/test_kasan.c:732
> [11:56:30] KASAN failure expected in "*(volatile char *)p", but none occurred
> [11:56:30] not ok 32 - kasan_global_oob_right
> [11:56:30] [FAILED] kasan_global_oob_right
> [11:56:30] # kasan_global_oob_left: EXPECTATION FAILED at lib/test_kasan.c:746
> [11:56:30] KASAN failure expected in "*(volatile char *)p", but none occurred
> [11:56:30] not ok 33 - kasan_global_oob_left
> [11:56:30] [FAILED] kasan_global_oob_left
>
> The global_oob_left test doesn't work on gcc either (but fails on all
> architectures, so is disabled), but kasan_global_oob_right should work
> in theory.
kasan_global_oob_right works for me with GCC, but it looks like
__asan_register_globals() never gets called when built with clang. This
fixes it:
diff --git a/arch/um/include/asm/common.lds.S b/arch/um/include/asm/common.lds.S
index 731f8c8422a2..fd481ac371de 100644
--- a/arch/um/include/asm/common.lds.S
+++ b/arch/um/include/asm/common.lds.S
@@ -84,6 +84,7 @@
.init_array : {
__init_array_start = .;
*(.kasan_init)
+ *(.init_array.*)
*(.init_array)
__init_array_end = .;
}
With that:
[13:12:15] =================== kasan (55 subtests) ====================
[13:12:15] [PASSED] kmalloc_oob_right
[13:12:15] [PASSED] kmalloc_oob_left
[13:12:15] [PASSED] kmalloc_node_oob_right
[13:12:15] [PASSED] kmalloc_pagealloc_oob_right
[13:12:15] [PASSED] kmalloc_pagealloc_uaf
[13:12:15] [PASSED] kmalloc_pagealloc_invalid_free
[13:12:15] [SKIPPED] pagealloc_oob_right
[13:12:15] [PASSED] pagealloc_uaf
[13:12:15] [PASSED] kmalloc_large_oob_right
[13:12:15] [PASSED] krealloc_more_oob
[13:12:15] [PASSED] krealloc_less_oob
[13:12:15] [PASSED] krealloc_pagealloc_more_oob
[13:12:15] [PASSED] krealloc_pagealloc_less_oob
[13:12:15] [PASSED] krealloc_uaf
[13:12:15] [PASSED] kmalloc_oob_16
[13:12:15] [PASSED] kmalloc_uaf_16
[13:12:15] [PASSED] kmalloc_oob_in_memset
[13:12:15] [PASSED] kmalloc_oob_memset_2
[13:12:15] [PASSED] kmalloc_oob_memset_4
[13:12:15] [PASSED] kmalloc_oob_memset_8
[13:12:15] [PASSED] kmalloc_oob_memset_16
[13:12:15] [PASSED] kmalloc_memmove_negative_size
[13:12:15] [PASSED] kmalloc_memmove_invalid_size
[13:12:15] [PASSED] kmalloc_uaf
[13:12:15] [PASSED] kmalloc_uaf_memset
[13:12:15] [PASSED] kmalloc_uaf2
[13:12:15] [PASSED] kfree_via_page
[13:12:15] [PASSED] kfree_via_phys
[13:12:15] [PASSED] kmem_cache_oob
[13:12:15] [PASSED] kmem_cache_accounted
[13:12:15] [PASSED] kmem_cache_bulk
[13:12:15] [PASSED] kasan_global_oob_right
[13:12:15] [PASSED] kasan_global_oob_left
[13:12:15] [PASSED] kasan_stack_oob
[13:12:15] [PASSED] kasan_alloca_oob_left
[13:12:15] [PASSED] kasan_alloca_oob_right
[13:12:15] [PASSED] ksize_unpoisons_memory
[13:12:15] [PASSED] ksize_uaf
[13:12:15] [PASSED] kmem_cache_double_free
[13:12:15] [PASSED] kmem_cache_invalid_free
[13:12:15] [PASSED] kmem_cache_double_destroy
[13:12:15] [PASSED] kasan_memchr
[13:12:15] [PASSED] kasan_memcmp
[13:12:15] [PASSED] kasan_strings
[13:12:15] [PASSED] kasan_bitops_generic
[13:12:15] [SKIPPED] kasan_bitops_tags
[13:12:15] [PASSED] kmalloc_double_kzfree
[13:12:15] [SKIPPED] vmalloc_helpers_tags
[13:12:15] [PASSED] vmalloc_oob
[13:12:15] [SKIPPED] vmap_tags
[13:12:15] [SKIPPED] vm_map_ram_tags
[13:12:15] [SKIPPED] vmalloc_percpu
[13:12:15] [SKIPPED] match_all_not_assigned
[13:12:15] [SKIPPED] match_all_ptr_tag
[13:12:15] [SKIPPED] match_all_mem_tag
[13:12:15] ====================== [PASSED] kasan ======================
[13:12:15] ============================================================
[13:12:15] Testing complete. Passed: 46, Failed: 0, Crashed: 0, Skipped: 9, Errors: 0
> > diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
> > index a4f07de21771..d8c518bd0e7d 100644
> > --- a/mm/kasan/shadow.c
> > +++ b/mm/kasan/shadow.c
> > @@ -295,8 +295,14 @@ int kasan_populate_vmalloc(unsigned long addr, unsigned long size)
> > return 0;
> >
> > shadow_start = (unsigned long)kasan_mem_to_shadow((void *)addr);
> > - shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
> > shadow_end = (unsigned long)kasan_mem_to_shadow((void *)addr + size);
> > +
> > + if (IS_ENABLED(CONFIG_UML)) {
> > + __memset(kasan_mem_to_shadow((void *)addr), KASAN_VMALLOC_INVALID, shadow_end - shadow_start);
> > + return 0;
> > + }
> > +
> > + shadow_start = ALIGN_DOWN(shadow_start, PAGE_SIZE);
> > shadow_end = ALIGN(shadow_end, PAGE_SIZE);
>
> Is there a particular reason we're not doing the rounding under UML,
> particularly since I think it's happening anyway in
> kasan_release_vmalloc() below. (I get that it's not really necessary,
> but is there an actual bug you've noticed with it?)
No, I didn't notice any bug.
> > ret = apply_to_page_range(&init_mm, shadow_start,
> > @@ -466,6 +472,10 @@ void kasan_release_vmalloc(unsigned long start, unsigned long end,
> >
> > if (shadow_end > shadow_start) {
> > size = shadow_end - shadow_start;
> > + if (IS_ENABLED(CONFIG_UML)) {
> > + __memset(shadow_start, KASAN_SHADOW_INIT, shadow_end - shadow_start);
> > + return;
> > + }
> > apply_to_existing_page_range(&init_mm,
> > (unsigned long)shadow_start,
> > size, kasan_depopulate_vmalloc_pte,
> > @@ -531,6 +541,11 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask)
> > if (WARN_ON(!PAGE_ALIGNED(shadow_start)))
> > return -EINVAL;
> >
> > + if (IS_ENABLED(CONFIG_UML)) {
> > + __memset((void *)shadow_start, KASAN_SHADOW_INIT, shadow_size);
> > + return 0;
> > + }
> > +
> > ret = __vmalloc_node_range(shadow_size, 1, shadow_start,
> > shadow_start + shadow_size,
> > GFP_KERNEL,
> > @@ -554,6 +569,9 @@ int kasan_alloc_module_shadow(void *addr, size_t size, gfp_t gfp_mask)
> >
> > void kasan_free_module_shadow(const struct vm_struct *vm)
> > {
> > + if (IS_ENABLED(CONFIG_UML))
> > + return;
> > +
> > if (vm->flags & VM_KASAN)
> > vfree(kasan_mem_to_shadow(vm->addr));
> > }
>
> In any case, this looks pretty great to me. I still definitely want to
> play with it a bit more, particularly with various module loads -- and
> it'd be great to track down why those global_oob tests are failing --
> but I'm definitely hopeful that we can finish this off and get it
> upstream.
>
> It's probably worth sending a new rebased/combined patch out which has
> your fixes and applies more cleanly on recent kernels. (I've got a
> working tree here, so I can do that if you'd prefer.)
Please feel free to do so. Thanks!
next prev parent reply other threads:[~2022-05-25 11:18 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-26 0:46 [PATCH] UML: add support for KASAN under x86_64 Patricia Alfonso
2020-02-26 1:19 ` Brendan Higgins
2020-02-26 15:24 ` Dmitry Vyukov
2020-03-06 0:03 ` Patricia Alfonso
2020-03-11 10:32 ` Johannes Berg
2020-03-11 10:46 ` Dmitry Vyukov
2020-03-11 11:18 ` Johannes Berg
2020-03-11 11:40 ` Johannes Berg
2020-03-11 17:34 ` Dmitry Vyukov
2020-03-20 13:39 ` Johannes Berg
2020-03-20 15:18 ` Dmitry Vyukov
2020-03-30 7:43 ` Johannes Berg
2020-03-30 8:38 ` Dmitry Vyukov
2020-03-30 8:41 ` Johannes Berg
2020-03-31 6:14 ` David Gow
2020-03-31 7:43 ` Johannes Berg
2020-03-31 16:39 ` Patricia Alfonso
2020-03-31 16:54 ` Richard Weinberger
2020-03-11 22:32 ` Patricia Alfonso
2020-03-11 22:44 ` Johannes Berg
2022-05-24 10:34 ` Vincent Whitchurch
2022-05-24 10:45 ` Johannes Berg
2022-05-24 19:35 ` David Gow
2022-05-25 11:17 ` Vincent Whitchurch [this message]
2022-05-26 1:01 ` [RFC PATCH v3] " David Gow
2022-05-26 9:29 ` Johannes Berg
2022-05-27 5:31 ` Dmitry Vyukov
2022-05-27 7:32 ` Johannes Berg
2022-05-27 10:36 ` Johannes Berg
2022-05-27 13:05 ` Johannes Berg
2022-05-27 13:09 ` Dmitry Vyukov
2022-05-27 13:15 ` Johannes Berg
2022-05-27 13:18 ` Dmitry Vyukov
2022-05-27 13:27 ` Johannes Berg
2022-05-27 13:52 ` Dmitry Vyukov
2022-05-27 14:27 ` Johannes Berg
2022-05-27 15:46 ` Dmitry Vyukov
2020-03-29 19:06 ` [PATCH] " Richard Weinberger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220525111756.GA15955@axis.com \
--to=vincent.whitchurch@axis.com \
--cc=anton.ivanov@cambridgegreys.com \
--cc=aryabinin@virtuozzo.com \
--cc=brendanhiggins@google.com \
--cc=davidgow@google.com \
--cc=dja@axtens.net \
--cc=dvyukov@google.com \
--cc=jdike@addtoit.com \
--cc=johannes@sipsolutions.net \
--cc=kasan-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-um@lists.infradead.org \
--cc=richard@nod.at \
--cc=trishalfonso@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).