On Wed, Apr 17, 2019 at 1:03 PM Alexander Potapenko <glider@xxxxxxxxxx> wrote: > > On Tue, Apr 16, 2019 at 6:30 PM Christopher Lameter <cl@xxxxxxxxx> wrote: > > > > On Tue, 16 Apr 2019, Alexander Potapenko wrote: > > > > > > Hmmm... But we already have debugging options that poison objects and > > > > pages? > > > Laura Abbott mentioned in one of the previous threads > > > (https://marc.info/?l=kernel-hardening&m=155474181528491&w=2) that: > > > > > > """ > > > I've looked at doing something similar in the past (failing to find > > > the thread this morning...) and while this will work, it has pretty > > > serious performance issues. It's not actually the poisoning which > > > is expensive but that turning on debugging removes the cpu slab > > > which has significant performance penalties. > > > > Ok you could rework that logic to be able to keep the per cpu slabs? > I'll look into that. There's a lot going on with checking those > poisoned bytes, although we don't need that for hardening. > > What do you think about the proposed approach to page initialization? > We could separate that part from slab poisoning. > > > Also if you do the zeroing then you need to do it in the hotpath. And this > > patch introduces new instructions to that hotpath for checking and > > executing the zeroing. > Right now the patch doesn't slow down the default case when > CONFIG_INIT_HEAP_ALL=n, as GFP_INIT_ALWAYS_ON is 0. > In the case heap initialization is enabled we could probably omit the > gfp_flags check, as it'll be always zero in the case there's a > constructor or RCU flag is set. > So we'll have two branches instead of one in the case CONFIG_INIT_HEAP_ALL=y. > Ok, I think we could do the same without extra branches. Right now I'm working on a patch that uses static branches in the function that checks GFP flags: static inline bool want_init_memory(gfp_t flags) { if (static_branch_unlikely(&init_allocations)) return true; return flags & __GFP_ZERO; } and does the following in slab_alloc_node(): if (unlikely(want_init_memory(gfpflags)) && object) s->poison_fn(s, object); , where s->poison_fn is either memset(object, 0, s->object_size) for normal SLAB caches or a no-op for SLAB caches that have ctors (I _think_ I don't have to special-case SLAB_TYPESAFE_BY_RCU). With init_allocations disabled this doesn't affect the kernel performance (hackbench shows negative slowdown within the standard deviation). Most certainly the indirect call is performed not too often. With init_allocations enabled this yields ~7% slowdown on hackbench. I believe most of that is caused by double initialization, which we can eliminate by passing an extra GFP flag to the page allocator. > > -- > 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
