On Tue, Jun 23, 2020 at 10:31 AM Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote: > > On Tue, Jun 23, 2020 at 9:24 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. 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
