On Tue, Jun 23, 2020 at 11:14 AM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: > > On Tue, Jun 23, 2020 at 10:38 AM Alexander Potapenko <glider@xxxxxxxxxx> 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. Thanks for clarifying! Then both KFENCE and init_on_free should work fine with that change. > > 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@xxxxxxxxxx> wrote: > > > > > > > > > > On Tue, 23 Jun 2020 at 08:45, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: > > > > > > > > > > > > On Tue, Jun 23, 2020 at 8:26 AM Jann Horn <jannh@xxxxxxxxxx> 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 -- 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
