Messages in this thread | | | From | Marco Elver <> | Date | Fri, 30 Oct 2020 14:46:40 +0100 | Subject | Re: [PATCH v6 6/9] kfence, kasan: make KFENCE compatible with KASAN |
| |
On Fri, 30 Oct 2020 at 03:50, Jann Horn <jannh@google.com> wrote: > > On Thu, Oct 29, 2020 at 2:17 PM Marco Elver <elver@google.com> wrote: > > We make KFENCE compatible with KASAN for testing KFENCE itself. In > > particular, KASAN helps to catch any potential corruptions to KFENCE > > state, or other corruptions that may be a result of freepointer > > corruptions in the main allocators. > > > > To indicate that the combination of the two is generally discouraged, > > CONFIG_EXPERT=y should be set. It also gives us the nice property that > > KFENCE will be build-tested by allyesconfig builds. > > > > Reviewed-by: Dmitry Vyukov <dvyukov@google.com> > > Co-developed-by: Marco Elver <elver@google.com> > > Signed-off-by: Marco Elver <elver@google.com> > > Signed-off-by: Alexander Potapenko <glider@google.com> > > Reviewed-by: Jann Horn <jannh@google.com>
Thanks!
> with one nit: > > [...] > > diff --git a/mm/kasan/common.c b/mm/kasan/common.c > [...] > > @@ -141,6 +142,14 @@ void kasan_unpoison_shadow(const void *address, size_t size) > > */ > > address = reset_tag(address); > > > > + /* > > + * We may be called from SL*B internals, such as ksize(): with a size > > + * not a multiple of machine-word size, avoid poisoning the invalid > > + * portion of the word for KFENCE memory. > > + */ > > + if (is_kfence_address(address)) > > + return; > > It might be helpful if you could add a comment that explains that > kasan_poison_object_data() does not need a similar guard because > kasan_poison_object_data() is always paired with > kasan_unpoison_object_data() - that threw me off a bit at first.
Well, KFENCE objects should never be poisoned/unpoisoned because the kasan_alloc and free hooks have a kfence guard, and none of the code in sl*b.c that does kasan_{poison,unpoison}_object_data() should be executed for KFENCE objects.
But I just noticed that kernel/scs.c seems to kasan_poison and unpoison objects, and keeps them poisoned for most of the object lifetime. I think we better add a kfence guard to kasan_poison_shadow() as well.
Thanks, -- Marco
| |