All of lore.kernel.org
 help / color / mirror / Atom feed
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!

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.