From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752555AbcDOIHa (ORCPT ); Fri, 15 Apr 2016 04:07:30 -0400 Received: from mail-wm0-f68.google.com ([74.125.82.68]:33126 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750928AbcDOIHW (ORCPT ); Fri, 15 Apr 2016 04:07:22 -0400 Date: Fri, 15 Apr 2016 10:07:15 +0200 From: Ingo Molnar To: Kees Cook Cc: Baoquan He , Yinghai Lu , Ard Biesheuvel , Matt Redfearn , x86@kernel.org, "H. Peter Anvin" , Ingo Molnar , Borislav Petkov , Vivek Goyal , Andy Lutomirski , lasse.collin@tukaani.org, Andrew Morton , Dave Young , kernel-hardening@lists.openwall.com, LKML , Linus Torvalds , Thomas Gleixner Subject: Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET Message-ID: <20160415080715.GD30715@gmail.com> References: <1460672954-32567-1-git-send-email-keescook@chromium.org> <1460672954-32567-4-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460672954-32567-4-git-send-email-keescook@chromium.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Kees Cook wrote: > From: Baoquan He > > Currently CONFIG_RANDOMIZE_BASE_MAX_OFFSET is used to limit the maximum > offset for kernel randomization. This limit doesn't need to be a CONFIG > since it is tied completely to KERNEL_IMAGE_SIZE, and will make no sense > once physical and virtual offsets are randomized separately. This patch > removes CONFIG_RANDOMIZE_BASE_MAX_OFFSET and consolidates the Kconfig > help text. > > Signed-off-by: Baoquan He > [kees: rewrote changelog, dropped KERNEL_IMAGE_SIZE_DEFAULT, moved earlier] > Signed-off-by: Kees Cook > --- > arch/x86/Kconfig | 57 +++++++++++++----------------------- > arch/x86/boot/compressed/aslr.c | 12 ++++---- > arch/x86/include/asm/page_64_types.h | 8 ++--- > arch/x86/mm/init_32.c | 3 -- > 4 files changed, 29 insertions(+), 51 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 2dc18605831f..fd9ac711ada8 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1936,51 +1936,36 @@ config RANDOMIZE_BASE > depends on RELOCATABLE > default n > ---help--- > - Randomizes the physical and virtual address at which the > - kernel image is decompressed, as a security feature that > - deters exploit attempts relying on knowledge of the location > - of kernel internals. > + Randomizes the physical address at which the kernel image > + is decompressed and the virtual address where the kernel > + image is mapped, as a secrurity feature that deters exploit Guys, please _read_ what you write: s/secrurity/security > + attempts relying on knowledge of the location of kernel > + internals. > + > + The kernel physical address can be randomized from 16M to > + 64T at most. The 64TB value sure reads weird if you are configuring a 32-bit system ... A much better approach would be to split the help text into 32-bit and 64-bit portions: On 64-bit systems the kernel physical address will be randomized from 16M to the top of available physical memory. (With a maximum of 64TB.) On 32-bit systems the kernel physical address will be randomized from 16MB to 1GB. Also note the assertive tone: if this Kconfig feature is eanbled, we say that the kernel address _will_ be randomized, and we should make sure it is. (If for some weird reason randomization fails we should warn prominently during bootup.) > The kernel virtual address will be offset > + by up to KERNEL_IMAGE_SIZE. On 32-bit KERNEL_IMAGE_SIZE is > + 512MiB. while on 64-bit this is limited by how the kernel > + fixmap page table is positioned, so this cannot be larger > + than 1GiB currently. Without RANDOMIZE_BASE there is a 512MiB > + to 1.5GiB split between kernel and modules. When RANDOMIZE_BASE > + is enabled, the modules area will shrink to compensate, up > + to a 1GiB to 1GiB split, KERNEL_IMAGE_SIZE changes from 512MiB > + to 1GiB. Beyond the broken capitalization, I'll show you what 99.999% of users who are not kernel hackers will understand from this paragraph, approximately: To dream: ay, and them? To bear to sling afterprises coil, and scover'd cowards of resolence dream: ay, the us for no mome wish'd. Thus and sweary life, or nobles cast and makes, whips and that is sicklied of resolence of so long afterprises us more; for whips all; and name whething after bear to sleep; to beart-ache shocks the undiscover'd consummative have, but that pith a sleep of somethe under 'tis the spurns of troud makes off thance doth make whose bourns of dispriz'd consient and arms more. So this is really deep kernel internals, I get a headache trying to interpret it, and it's my job to interpret this! Please try to formulate key pieces of information in Kconfig help texts in a more ... approachable fashion, and move the jargon to .c source code files. > Entropy is generated using the RDRAND instruction if it is > supported. If RDTSC is supported, it is used as well. If > neither RDRAND nor RDTSC are supported, then randomness is > read from the i8254 timer. Also, instead of 'used as well' I'd say "is mixed into the entropy pool as well" or so, to make sure it's clear that we don't exclusively rely on RDRAND or RDTSC. Also, could we always mix the i8254 timer into this as well, not just when RDTSC is unavailable? > - The kernel will be offset by up to RANDOMIZE_BASE_MAX_OFFSET, > - and aligned according to PHYSICAL_ALIGN. Since the kernel is > - built using 2GiB addressing, and PHYSICAL_ALGIN must be at a > - minimum of 2MiB, only 10 bits of entropy is theoretically > - possible. At best, due to page table layouts, 64-bit can use > - 9 bits of entropy and 32-bit uses 8 bits. > + Since the kernel is built using 2GiB addressing, and > + PHYSICAL_ALGIN must be at a minimum of 2MiB, only 10 bits of > + entropy is theoretically possible. At best, due to page table > + layouts, 64-bit can use 9 bits of entropy and 32-bit uses 8 > + bits. Please read what you write, there's a typo in this section. Another request: please stop the MiB/GiB nonsense and call it MB/GB. This isn't storage code that has to fight marketing lies. Only the KASLR section in arch/x86/Kconfig* is using MiB/GiB, everything else uses MB/GB naming, we should stick with that. Thanks, Ingo From mboxrd@z Thu Jan 1 00:00:00 1970 Reply-To: kernel-hardening@lists.openwall.com Sender: Ingo Molnar Date: Fri, 15 Apr 2016 10:07:15 +0200 From: Ingo Molnar Message-ID: <20160415080715.GD30715@gmail.com> References: <1460672954-32567-1-git-send-email-keescook@chromium.org> <1460672954-32567-4-git-send-email-keescook@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1460672954-32567-4-git-send-email-keescook@chromium.org> Subject: [kernel-hardening] Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET To: Kees Cook Cc: Baoquan He , Yinghai Lu , Ard Biesheuvel , Matt Redfearn , x86@kernel.org, "H. Peter Anvin" , Ingo Molnar , Borislav Petkov , Vivek Goyal , Andy Lutomirski , lasse.collin@tukaani.org, Andrew Morton , Dave Young , kernel-hardening@lists.openwall.com, LKML , Linus Torvalds , Thomas Gleixner List-ID: * Kees Cook wrote: > From: Baoquan He > > Currently CONFIG_RANDOMIZE_BASE_MAX_OFFSET is used to limit the maximum > offset for kernel randomization. This limit doesn't need to be a CONFIG > since it is tied completely to KERNEL_IMAGE_SIZE, and will make no sense > once physical and virtual offsets are randomized separately. This patch > removes CONFIG_RANDOMIZE_BASE_MAX_OFFSET and consolidates the Kconfig > help text. > > Signed-off-by: Baoquan He > [kees: rewrote changelog, dropped KERNEL_IMAGE_SIZE_DEFAULT, moved earlier] > Signed-off-by: Kees Cook > --- > arch/x86/Kconfig | 57 +++++++++++++----------------------- > arch/x86/boot/compressed/aslr.c | 12 ++++---- > arch/x86/include/asm/page_64_types.h | 8 ++--- > arch/x86/mm/init_32.c | 3 -- > 4 files changed, 29 insertions(+), 51 deletions(-) > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 2dc18605831f..fd9ac711ada8 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1936,51 +1936,36 @@ config RANDOMIZE_BASE > depends on RELOCATABLE > default n > ---help--- > - Randomizes the physical and virtual address at which the > - kernel image is decompressed, as a security feature that > - deters exploit attempts relying on knowledge of the location > - of kernel internals. > + Randomizes the physical address at which the kernel image > + is decompressed and the virtual address where the kernel > + image is mapped, as a secrurity feature that deters exploit Guys, please _read_ what you write: s/secrurity/security > + attempts relying on knowledge of the location of kernel > + internals. > + > + The kernel physical address can be randomized from 16M to > + 64T at most. The 64TB value sure reads weird if you are configuring a 32-bit system ... A much better approach would be to split the help text into 32-bit and 64-bit portions: On 64-bit systems the kernel physical address will be randomized from 16M to the top of available physical memory. (With a maximum of 64TB.) On 32-bit systems the kernel physical address will be randomized from 16MB to 1GB. Also note the assertive tone: if this Kconfig feature is eanbled, we say that the kernel address _will_ be randomized, and we should make sure it is. (If for some weird reason randomization fails we should warn prominently during bootup.) > The kernel virtual address will be offset > + by up to KERNEL_IMAGE_SIZE. On 32-bit KERNEL_IMAGE_SIZE is > + 512MiB. while on 64-bit this is limited by how the kernel > + fixmap page table is positioned, so this cannot be larger > + than 1GiB currently. Without RANDOMIZE_BASE there is a 512MiB > + to 1.5GiB split between kernel and modules. When RANDOMIZE_BASE > + is enabled, the modules area will shrink to compensate, up > + to a 1GiB to 1GiB split, KERNEL_IMAGE_SIZE changes from 512MiB > + to 1GiB. Beyond the broken capitalization, I'll show you what 99.999% of users who are not kernel hackers will understand from this paragraph, approximately: To dream: ay, and them? To bear to sling afterprises coil, and scover'd cowards of resolence dream: ay, the us for no mome wish'd. Thus and sweary life, or nobles cast and makes, whips and that is sicklied of resolence of so long afterprises us more; for whips all; and name whething after bear to sleep; to beart-ache shocks the undiscover'd consummative have, but that pith a sleep of somethe under 'tis the spurns of troud makes off thance doth make whose bourns of dispriz'd consient and arms more. So this is really deep kernel internals, I get a headache trying to interpret it, and it's my job to interpret this! Please try to formulate key pieces of information in Kconfig help texts in a more ... approachable fashion, and move the jargon to .c source code files. > Entropy is generated using the RDRAND instruction if it is > supported. If RDTSC is supported, it is used as well. If > neither RDRAND nor RDTSC are supported, then randomness is > read from the i8254 timer. Also, instead of 'used as well' I'd say "is mixed into the entropy pool as well" or so, to make sure it's clear that we don't exclusively rely on RDRAND or RDTSC. Also, could we always mix the i8254 timer into this as well, not just when RDTSC is unavailable? > - The kernel will be offset by up to RANDOMIZE_BASE_MAX_OFFSET, > - and aligned according to PHYSICAL_ALIGN. Since the kernel is > - built using 2GiB addressing, and PHYSICAL_ALGIN must be at a > - minimum of 2MiB, only 10 bits of entropy is theoretically > - possible. At best, due to page table layouts, 64-bit can use > - 9 bits of entropy and 32-bit uses 8 bits. > + Since the kernel is built using 2GiB addressing, and > + PHYSICAL_ALGIN must be at a minimum of 2MiB, only 10 bits of > + entropy is theoretically possible. At best, due to page table > + layouts, 64-bit can use 9 bits of entropy and 32-bit uses 8 > + bits. Please read what you write, there's a typo in this section. Another request: please stop the MiB/GiB nonsense and call it MB/GB. This isn't storage code that has to fight marketing lies. Only the KASLR section in arch/x86/Kconfig* is using MiB/GiB, everything else uses MB/GB naming, we should stick with that. Thanks, Ingo