linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Dmitry Vyukov <dvyukov@google.com>
To: Alexander Potapenko <glider@google.com>
Cc: Marco Elver <elver@google.com>, Jann Horn <jannh@google.com>,
	 Kernel Hardening <kernel-hardening@lists.openwall.com>,
	Christoph Lameter <cl@linux.com>,
	 Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	 Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 Linux-MM <linux-mm@kvack.org>,
	Andrey Konovalov <andreyknvl@google.com>,
	 Will Deacon <will@kernel.org>,
	kasan-dev <kasan-dev@googlegroups.com>,
	 Kees Cook <keescook@google.com>
Subject: Re: Kernel hardening project suggestion: Normalizing ->ctor slabs and TYPESAFE_BY_RCU slabs
Date: Tue, 23 Jun 2020 11:14:19 +0200	[thread overview]
Message-ID: <CACT4Y+ZHoQ5ZPfsvaiQMXrrTxv9-LgP+v_o5Ah2gFBwqQjv-+g@mail.gmail.com> (raw)
In-Reply-To: <CAG_fn=VEb7XYwi0ZnOXRx-Yss++OhnpKCO-7tFvCOp4pi4MLcA@mail.gmail.com>

On Tue, Jun 23, 2020 at 10:38 AM Alexander Potapenko <glider@google.com> wrote:
> > > KFENCE also has to ignore both TYPESAFE_BY_RCU and ctors.
> > > For ctors it should be pretty straightforward to fix (and won't
> > > require any changes to SL[AU]B). Not sure if your proposal for RCU
> > > will also work for KFENCE.
> >
> > Does it work for objects freed by call_rcu in normal slabs?
> > If yes, then I would assume it will work for TYPESAFE_BY_RCU after
> > this change, or is there a difference?
>
> If my understanding is correct, TYPESAFE_BY_RCU means that the object
> may be used after it has been freed, that's why we cannot further
> reuse or wipe it before ensuring they aren't used anymore.

Yes, but only within an rcu grace period.
And this proposal will take care of this: from the point of view of
slab, the object is freed after an additional rcu grace period. So
when it reaches slab free, it must not be used anymore.

> Objects allocated from normal slabs cannot be used after they've been
> freed, so I don't see how this change applies to them.
>
> > > Another beneficiary of RCU/ctor normalization would be
> > > init_on_alloc/init_on_free, which also ignore such slabs.
> > >
> > > On Tue, Jun 23, 2020 at 9:18 AM Marco Elver <elver@google.com> wrote:
> > > >
> > > > On Tue, 23 Jun 2020 at 08:45, Dmitry Vyukov <dvyukov@google.com> wrote:
> > > > >
> > > > > On Tue, Jun 23, 2020 at 8:26 AM Jann Horn <jannh@google.com> wrote:
> > > > > >
> > > > > > Hi!
> > > > > >
> > > > > > Here's a project idea for the kernel-hardening folks:
> > > > > >
> > > > > > The slab allocator interface has two features that are problematic for
> > > > > > security testing and/or hardening:
> > > > > >
> > > > > >  - constructor slabs: These things come with an object constructor
> > > > > > that doesn't run when an object is allocated, but instead when the
> > > > > > slab allocator grabs a new page from the page allocator. This is
> > > > > > problematic for use-after-free detection mechanisms such as HWASAN and
> > > > > > Memory Tagging, which can only do their job properly if the address of
> > > > > > an object is allowed to change every time the object is
> > > > > > freed/reallocated. (You can't change the address of an object without
> > > > > > reinitializing the entire object because e.g. an empty list_head
> > > > > > points to itself.)
> > > > > >
> > > > > >  - RCU slabs: These things basically permit use-after-frees by design,
> > > > > > and stuff like ASAN/HWASAN/Memory Tagging essentially doesn't work on
> > > > > > them.
> > > > > >
> > > > > >
> > > > > > It would be nice to have a config flag or so that changes the SLUB
> > > > > > allocator's behavior such that these slabs can be instrumented
> > > > > > properly. Something like:
> > > > > >
> > > > > >  - Let calculate_sizes() reserve space for an rcu_head on each object
> > > > > > in TYPESAFE_BY_RCU slabs, make kmem_cache_free() redirect to
> > > > > > call_rcu() for these slabs, and remove most of the other
> > > > > > special-casing, so that KASAN can instrument these slabs.
> > > > > >  - For all constructor slabs, let slab_post_alloc_hook() call the
> > > > > > ->ctor() function on each allocated object, so that Memory Tagging and
> > > > > > HWASAN will work on them.
> > > > >
> > > > > Hi Jann,
> > > > >
> > > > > Both things sound good to me. I think we considered doing the ctor's
> > > > > change with KASAN, but we did not get anywhere. The only argument
> > > > > against it I remember now was "performance", but it's not that
> > > > > important if this mode is enabled only with KASAN and other debugging
> > > > > tools. Performance is definitely not as important as missing bugs. The
> > > > > additional code complexity for ctors change should be minimal.
> > > > > The rcu change would also be useful, but I would assume it will be larger.
> > > > > Please add them to [1], that's KASAN laundry list.
> > > > >
> > > > > +Alex, Marco, will it be useful for KFENCE [2] as well? Do ctors/rcu
> > > > > affect KFENCE? Will we need any special handling for KFENCE?
> > > > > I assume it will also be useful for KMSAN b/c we can re-mark objects
> > > > > as uninitialized only after they have been reallocated.
> > > >
> > > > Yes, we definitely need to handle TYPESAFE_BY_RCU.
> > > >
> > > > > [1] https://bugzilla.kernel.org/buglist.cgi?bug_status=__open__&component=Sanitizers&list_id=1063981&product=Memory%20Management
> > > > > [2] https://github.com/google/kasan/commits/kfence
> > >
> > >
> > >
> > > --
> > > Alexander Potapenko
> > > Software Engineer
> > >
> > > Google Germany GmbH
> > > Erika-Mann-Straße, 33
> > > 80636 München
> > >
> > > Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> > > Registergericht und -nummer: Hamburg, HRB 86891
> > > Sitz der Gesellschaft: Hamburg
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg


  reply	other threads:[~2020-06-23  9:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-23  6:26 Kernel hardening project suggestion: Normalizing ->ctor slabs and TYPESAFE_BY_RCU slabs Jann Horn
2020-06-23  6:45 ` Dmitry Vyukov
2020-06-23  7:17   ` Marco Elver
2020-06-23  7:24     ` Alexander Potapenko
2020-06-23  8:31       ` Dmitry Vyukov
2020-06-23  8:38         ` Alexander Potapenko
2020-06-23  9:14           ` Dmitry Vyukov [this message]
2020-06-23  9:23             ` Alexander Potapenko
2023-08-25 21:22 ` Jann Horn

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=CACT4Y+ZHoQ5ZPfsvaiQMXrrTxv9-LgP+v_o5Ah2gFBwqQjv-+g@mail.gmail.com \
    --to=dvyukov@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=andreyknvl@google.com \
    --cc=cl@linux.com \
    --cc=elver@google.com \
    --cc=glider@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=jannh@google.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=keescook@google.com \
    --cc=kernel-hardening@lists.openwall.com \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=will@kernel.org \
    /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).