Date: Mon, 24 Sep 2018 18:58:44 +0000 From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com> To: "keescook@...omium.org" <keescook@...omium.org> CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "daniel@...earbox.net" <daniel@...earbox.net>, "jannh@...gle.com" <jannh@...gle.com>, "arjan@...ux.intel.com" <arjan@...ux.intel.com>, "tglx@...utronix.de" <tglx@...utronix.de>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "x86@...nel.org" <x86@...nel.org>, "kristen@...ux.intel.com" <kristen@...ux.intel.com>, "hpa@...or.com" <hpa@...or.com>, "mingo@...hat.com" <mingo@...hat.com>, "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>, "kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>, "Hansen, Dave" <dave.hansen@...el.com> Subject: Re: [PATCH v6 3/4] vmalloc: Add debugfs modfraginfo On Fri, 2018-09-21 at 11:56 -0700, Kees Cook wrote: > On Thu, Sep 13, 2018 at 2:31 PM, Rick Edgecombe > <rick.p.edgecombe@...el.com> wrote: > > +done: > > + gap = (MODULES_END - last_end); > > + if (gap > largest_free) > > + largest_free = gap; > > + total_free += gap; > > + > > + spin_unlock(&vmap_area_lock); > > + > > + seq_printf(m, "\tLargest free space:\t%lu kB\n", largest_free / > > 1024); > > + seq_printf(m, "\t Total free space:\t%lu kB\n", total_free / 1024); > > + > > + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && kaslr_enabled()) > > + seq_printf(m, "Allocations in backup area:\t%lu\n", > > backup_cnt); > I don't think the IS_ENABLED is needed here? The reason for this is that for ARCH=um, CONFIG_X86_64 is defined but kaslr_enabled is not. kaslr_enabled is declared above to protect against a compiler error. So IS_ENABLED(CONFIG_RANDOMIZE_BASE) is protecting kaslr_enabled from causing a linker error. It gets constant evaluated to 0 and the compiler optimizes out the kaslr_enabled call. Thought it was better to guard with CONFIG_RANDOMIZE_BASE than with CONFIG_UM, to try to catch the broader situation. I guess I could move it to a helper inside ifdefs instead. Was trying to keep the ifdef-ed code down. > I wonder if there is a better way to arrange this code that uses fewer > ifdefs, etc. Maybe a single CONFIG that capture whether or not > fine-grained module randomization is built in, like: > > config RANDOMIZE_FINE_MODULE > def_bool y if RANDOMIZE_BASE && X86_64 > > #ifdef CONFIG_RANDOMIZE_FINE_MODULE > ... > #endif > > But that doesn't capture the DEBUG_FS and PROC_FS bits ... so ... > maybe not worth it. I guess, either way: Hmmm, didn't know about that. Would clean it up some at least. I wish the debugfs info could be in module.c to help with this IFDEFs, but it needs vmalloc internals. MODULES_VADDR is not standardized across the ARCH's as well, so this was my best attempt to implement this without having to make changes in other architectures. > Reviewed-by: Kees Cook <keescook@...omium.org> > > -Kees >
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.