Thanks for the review! I'll let Thomas address the feedback, though I've got some thoughts below on naming.
On Fri, Jun 17, 2016 at 3:26 AM, Ingo Molnar <[email protected]> wrote: > > * Kees Cook <[email protected]> wrote: > >> --- a/arch/x86/Kconfig >> +++ b/arch/x86/Kconfig >> @@ -1993,6 +1993,23 @@ config PHYSICAL_ALIGN >> >> Don't change this unless you know what you are doing. >> >> +config RANDOMIZE_MEMORY >> + bool "Randomize the kernel memory sections" >> + depends on X86_64 >> + depends on RANDOMIZE_BASE >> + default RANDOMIZE_BASE >> + ---help--- >> + Randomizes the base virtual address of kernel memory sections >> + (physical memory mapping, vmalloc & vmemmap). This security feature >> + makes exploits relying on predictable memory locations less >> reliable. >> + >> + The order of allocations remains unchanged. Entropy is generated in >> + the same way as RANDOMIZE_BASE. Current implementation in the >> optimal >> + configuration have in average 30,000 different possible virtual >> + addresses for each memory section. >> + >> + If unsure, say N. > > So this is really poor naming! I think "RANDOMIZE" should probably be changed, yes, though I have some caveats... > The user, in first approximation, will see something like this: > > Randomize the kernel memory sections (RANDOMIZE_MEMORY) [Y/n/?] (NEW) > > ... and could naively conclude that this feature will randomize memory > contents. > > Furthermore, there's no apparent connection to another, related kernel > feature: > > Randomize the address of the kernel image (KASLR) (RANDOMIZE_BASE) [Y/n/?] > > A better naming would be something like: > > Randomize other static kernel memory addresses (RANDOMIZE_ALL) [Y/n/?] > (NEW) > > (assuming it's truly 'all') There's always more to be identified and added. A few thoughts: "Address Space Layout Randomization" is a recognizable name, though I tried to avoid it in the descriptive texts because CONFIG_RANDOMIZE_BASE does not randomize "everything" and it doesn't randomize link order, etc, it only provides a randomized base address to the kernel text. Now, this is in line with all the userspace ASLR: each one of stack, mmap, brk, and text have their ASLR done via base address offsets. ASLR is a technique, and is frequently confused with "coverage", which I have been trying to fix. i.e. Kernel ASLR of text base address is one piece of coverage, but it leaves other things not yet ASLRed. So, now that we have something else besides base text address ASLR, I'm happy to create a configs that carry the name "KASLR". > But ... I don't see it explained anywhere why a user, once he expressed > interest > in randomization would even want to _not_ randomize as much as possible. I.e. > why > does this new option exist at all, shouldn't we just gradually extend > randomization to more memory areas and control it via CONFIG_RANDOMIZE_BASE? I think it's important to maintain a higher granularity of CONFIG options since the maturity of each given feature will vary from arch to arch. Also, it becomes hard to compare, for example, an x86 .config with an arm64 .config to see which features are enabled. If CONFIG_RANDOMIZE_MEMORY landed, it'd be easy to see that it is missing on arm64 from looking at .config files. Now, that said, a lot of people have wanted a "make this build as secure as possible" CONFIG, so it could be nice to have a CONFIG_KASLR that selects each of the available sub-configs. > Btw., CONFIG_RANDOMIZE_BASE is probably a misnomer, and after these patches it > will be more of a misnomer - so we should rename it to something better. For > example CONFIG_KASLR would have the advantage of being obviously named at a > glance, to anyone who knows what 'ASLR' means. To those who don't know the > short > description will tell it: > > Kernel address space layout randomization (KASLR) [Y/n/?] > > This would also fit the kernel internal naming of kaslr.c/etc. > > What do you think? How about something like this: CONFIG_KASLR depends on HAVE_ARCH_KASLR_TEXT || HAVE_ARCH_KASLR_MEMORY CONFIG_KASLR_TEXT depends on CONFIG_KASLR && HAVE_ARCH_KASLR_TEXT default CONFIG_KASLR CONFIG_KASLR_MEMORY depends on CONFIG_KASLR && HAVE_ARCH_KASLR_MEMORY default CONFIG_KASLR And then we can select HAVE_ARCH_KASLR_TEXT for x86, arm64, and MIPS, and when this lands, x86 would gain HAVE_ARCH_KASLR_MEMORY. -Kees -- Kees Cook Chrome OS & Brillo Security
