Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1379329 > unrolled thread

[PATCH v5 00/21] x86, boot: KASLR cleanup and 64-bit improvements

Back to article view | Back to linux.kernel

Contents

  [PATCH v5 00/21] x86, boot: KASLR cleanup and 64-bit improvements Kees Cook <keescook@chromium.org> - 2016-04-15 00:30 +0200
    [PATCH v5 05/21] x86, boot: Calculate decompression size during boot not build Kees Cook <keescook@chromium.org> - 2016-04-15 00:30 +0200
      Re: [PATCH v5 05/21] x86, boot: Calculate decompression size during  boot not build Ingo Molnar <mingo@kernel.org> - 2016-04-15 10:20 +0200
        Re: [PATCH v5 05/21] x86, boot: Calculate decompression size during  boot not build Kees Cook <keescook@chromium.org> - 2016-04-15 21:20 +0200
    [PATCH v5 21/21] x86, KASLR: Allow randomization below load address Kees Cook <keescook@chromium.org> - 2016-04-15 00:30 +0200
    [PATCH v5 09/21] x86, KASLR: Correctly bounds-check relocations Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 20/21] x86, KASLR: Remove unused slot tracking code Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 01/21] x86, KASLR: Remove unneeded boot_params argument Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 01/21] x86, KASLR: Remove unneeded boot_params argument Ingo Molnar <mingo@kernel.org> - 2016-04-15 09:30 +0200
        Re: [PATCH v5 01/21] x86, KASLR: Remove unneeded boot_params argument Kees Cook <keescook@chromium.org> - 2016-04-15 21:00 +0200
    [PATCH v5 19/21] x86, KASLR: Add physical address randomization >4G Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 13/21] x86, boot: Report overlap failures in memcpy Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 13/21] x86, boot: Report overlap failures in memcpy Lasse Collin <lasse.collin@tukaani.org> - 2016-04-15 17:00 +0200
        Re: [PATCH v5 13/21] x86, boot: Report overlap failures in memcpy Kees Cook <keescook@chromium.org> - 2016-04-15 21:30 +0200
    [PATCH v5 02/21] x86, KASLR: Handle kernel relocation above 2G Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 02/21] x86, KASLR: Handle kernel relocation above 2G Ingo Molnar <mingo@kernel.org> - 2016-04-15 09:50 +0200
        Re: [PATCH v5 02/21] x86, KASLR: Handle kernel relocation above 2G Kees Cook <keescook@chromium.org> - 2016-04-15 21:10 +0200
    [PATCH v5 16/21] x86, KASLR: Add virtual address choosing function Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 15/21] x86, KASLR: Add slot_area support functions Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 04/21] x86, boot: Move compressed kernel to end of decompression buffer Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of  decompression buffer Ingo Molnar <mingo@kernel.org> - 2016-04-15 10:20 +0200
        Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of  decompression buffer Kees Cook <keescook@chromium.org> - 2016-04-18 19:00 +0200
      Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of  decompression buffer Ingo Molnar <mingo@kernel.org> - 2016-04-15 11:10 +0200
    [PATCH v5 12/21] x86, 64bit: Set ident_mapping for KASLR Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 08/21] x86, KASLR: Clean up unused code from old run_size Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 14/21] x86, KASLR: Add slot_area to manage random slots Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 17/21] x86, KASLR: Clarify purpose of each get_random_long Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 11/21] x86, boot: Split out kernel_ident_mapping_init Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 10/21] x86, KASLR: Consolidate mem_avoid entries Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
    [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 03/21] x86, KASLR: Drop  CONFIG_RANDOMIZE_BASE_MAX_OFFSET Ingo Molnar <mingo@kernel.org> - 2016-04-15 10:10 +0200
        Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET Kees Cook <keescook@chromium.org> - 2016-04-15 21:20 +0200
          Re: [PATCH v5 03/21] x86, KASLR: Drop  CONFIG_RANDOMIZE_BASE_MAX_OFFSET Ingo Molnar <mingo@kernel.org> - 2016-04-16 10:50 +0200
    [PATCH v5 06/21] x86, KASLR: Update description for decompressor worst case size Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 06/21] x86, KASLR: Update description for  decompressor worst case size Lasse Collin <lasse.collin@tukaani.org> - 2016-04-15 18:20 +0200
    [PATCH v5 07/21] x86, boot: Fix run_size calculation Kees Cook <keescook@chromium.org> - 2016-04-15 00:40 +0200
      Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation Ingo Molnar <mingo@kernel.org> - 2016-04-15 10:40 +0200
        Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation Kees Cook <keescook@chromium.org> - 2016-04-15 21:30 +0200
          Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation Ingo Molnar <mingo@kernel.org> - 2016-04-16 11:10 +0200

Page 2 of 2 — ← Prev page 1 [2]

#1379535 — Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of decompression buffer

* Kees Cook <keescook@chromium.org> wrote:

> From: Yinghai Lu <yinghai@kernel.org>
> 
> This change makes later calculations about where the kernel is located
> easier to reason about. To better understand this change, we must first
> clarify what VO and ZO are. They were introduced in commits by hpa:
> 
> 77d1a49 x86, boot: make symbols from the main vmlinux available
> 37ba7ab x86, boot: make kernel_alignment adjustable; new bzImage fields
> 
> Specifically:
> 
> VO:
> - uncompressed kernel image
> - size: VO__end - VO__text ("VO_INIT_SIZE" define)
> 
> ZO:
> - bootable compressed kernel image (boot/compressed/vmlinux)
> - head text + compressed kernel (VO and relocs table) + decompressor code
> - size: ZO__end - ZO_startup_32 ("ZO_INIT_SIZE" define, though see below)
> 
> The INIT_SIZE definition is used to find the larger of the two image sizes:
> 
>  #define ZO_INIT_SIZE    (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
>  #define VO_INIT_SIZE    (VO__end - VO__text)
>  #if ZO_INIT_SIZE > VO_INIT_SIZE
>  #define INIT_SIZE ZO_INIT_SIZE
>  #else
>  #define INIT_SIZE VO_INIT_SIZE
>  #endif

Please also harmonize all the prefixes, i.e. use VO__/ZO__ everywhere (rename 
things where necessary), instead of this mixed up VO_/ZO_/VO__/ZO__ mess.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1381912 — Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of decompression buffer

On Fri, Apr 15, 2016 at 1:09 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>> This change makes later calculations about where the kernel is located
>> easier to reason about. To better understand this change, we must first
>> clarify what VO and ZO are. They were introduced in commits by hpa:
>>
>> 77d1a49 x86, boot: make symbols from the main vmlinux available
>> 37ba7ab x86, boot: make kernel_alignment adjustable; new bzImage fields
>>
>> Specifically:
>>
>> VO:
>> - uncompressed kernel image
>> - size: VO__end - VO__text ("VO_INIT_SIZE" define)
>>
>> ZO:
>> - bootable compressed kernel image (boot/compressed/vmlinux)
>> - head text + compressed kernel (VO and relocs table) + decompressor code
>> - size: ZO__end - ZO_startup_32 ("ZO_INIT_SIZE" define, though see below)
>>
>> The INIT_SIZE definition is used to find the larger of the two image sizes:
>>
>>  #define ZO_INIT_SIZE    (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
>>  #define VO_INIT_SIZE    (VO__end - VO__text)
>>  #if ZO_INIT_SIZE > VO_INIT_SIZE
>>  #define INIT_SIZE ZO_INIT_SIZE
>>  #else
>>  #define INIT_SIZE VO_INIT_SIZE
>>  #endif
>
> Please also harmonize all the prefixes, i.e. use VO__/ZO__ everywhere (rename
> things where necessary), instead of this mixed up VO_/ZO_/VO__/ZO__ mess.

I'm going to leave these as they are: they're auto-generated based on
various functions of interest:

boot/Makefile:sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW]
\(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|z_.*\)$$/\#define
ZO_\2 0x\1/p'

i.e. ZO__end is _end's location. ZO_input_data is input_data's
position. I think it would further complicate things if we tried to
consolidate consecutive "_"s, or if we eliminated the leading "_"
(ZOinput_data and ZO_end).

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1379578 — Re: [PATCH v5 04/21] x86, boot: Move compressed kernel to end of decompression buffer

* Kees Cook <keescook@chromium.org> wrote:

> When INIT_SIZE is bigger than VO_INIT_SIZE (uncommon but possible),
> the copied ZO occupies the memory from extract_offset to the end of
> decompression buffer. It overlaps with the soon-to-be-uncompressed kernel
> like this:
> 
>                             |-----compressed kernel image------|
>                             V                                  V
> 0                       extract_offset                      +INIT_SIZE
> |-----------|---------------|-------------------------|--------|
>             |               |                         |        |
>           VO__text      startup_32 of ZO          VO__end    ZO__end
>             ^                                         ^
>             |-------uncompressed kernel image---------|
> 
> When INIT_SIZE is equal to VO_INIT_SIZE (likely) there's still space
> left from end of ZO to the end of decompressing buffer, like below.
> 
>                             |-compressed kernel image-|
>                             V                         V
> 0                       extract_offset                      +INIT_SIZE
> |-----------|---------------|-------------------------|--------|
>             |               |                         |        |
>           VO__text      startup_32 of ZO          ZO__end    VO__end
>             ^                                                  ^
>             |------------uncompressed kernel image-------------|
> 
> To simplify calculations and avoid special cases, it is cleaner to
> always place the compressed kernel image in memory so that ZO__end
> is at the end of the decompression buffer, instead of placing that
> start extract_offset as is currently done.

Btw., it would be nice to also put such a visualization (of the current layout of 
these values) into the code itself.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1379347 — [PATCH v5 12/21] x86, 64bit: Set ident_mapping for KASLR

From: Yinghai Lu <yinghai@kernel.org>

Currently KASLR only supports relocation in a small physical range (from
16M to 1G), due to using the initial kernel page table identify mapping.
To support ranges above this, we need to have an identity mapping for
the desired memory range before we can decompress (and later run) the
kernel.

This patch adds an identity mapping for needed memory range. This happens
in two possible boot paths:

If loaded via startup_32, we need to set up the identity mapping we need.

If loaded from a 64-bit bootloader, the bootloader will have
already set up an identity mapping, and we'll start via ZO's
(arch/x86/boot/compressed/vmlinux) startup_64. In this case, the
bootloader's page tables need to be avoided when we select the new VO
(vmlinux) location. If not, the decompressor could overwrite them during
decompression.

To accomplish avoiding the bootloader's page tables, we could walk the
pagetable and find every page that is used, and add them to mem_avoid,
but this needs extra code and will require increasing the size of the
mem_avoid array.

Instead, we can create new ident mapping instead, and pages for the
pagetable will come from the _pagetable section of ZO, which means they
are already in mem_avoid array. To do this, we reuse the code for the
kernel's identity mapping.

The _pgtable will be shared by 32-bit and 64-bit path to reduce init_size,
as now ZO _rodata to _end will contribute init_size.

To handle the possible mappings, we need to increase the pgt buffer size:

When booting via startup_64, as we need to cover the old VO, params,
cmdline and new VO. In an extreme case we could have them all beyond the
512G boundary, which needs (2+2)*4 pages with 2M mappings. And we'll
need 2 for first 2M for VGA RAM. One more is needed for level4. This
gets us to 19 pages total.

When booting via startup_32, KASLR could move new VO above 4G, so we
need to set extra identity mappings for new VO, which should only need
(2+2) pages at most when it is beyond the 512G boundary. So 19 pages
is sufficient for this case as well.

Cc: Kees Cook <keescook@chromium.org>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/Makefile   |  3 ++
 arch/x86/boot/compressed/aslr.c     | 14 ++++++
 arch/x86/boot/compressed/head_64.S  |  4 +-
 arch/x86/boot/compressed/misc.h     | 11 +++++
 arch/x86/boot/compressed/misc_pgt.c | 93 +++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/boot.h         | 19 ++++++++
 6 files changed, 142 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/boot/compressed/misc_pgt.c

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index aa421126556d..af9ea6f0baa2 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -63,6 +63,9 @@ vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
 
 vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o
 vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/aslr.o
+ifdef CONFIG_X86_64
+	vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/misc_pgt.o
+endif
 
 $(obj)/eboot.o: KBUILD_CFLAGS += -fshort-wchar -mno-red-zone
 
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 277c7a4ec3a3..cd261debead9 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -194,6 +194,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	 */
 	mem_avoid[0].start = input;
 	mem_avoid[0].size = (output + init_size) - input;
+	fill_pagetable(mem_avoid[0].start, mem_avoid[0].size);
 
 	/* Avoid initrd. */
 	initrd_start  = (u64)real_mode->ext_ramdisk_image << 32;
@@ -202,6 +203,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	initrd_size |= real_mode->hdr.ramdisk_size;
 	mem_avoid[1].start = initrd_start;
 	mem_avoid[1].size = initrd_size;
+	/* No need to set mapping for initrd */
 
 	/* Avoid kernel command line. */
 	cmd_line  = (u64)real_mode->ext_cmd_line_ptr << 32;
@@ -212,10 +214,19 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 		;
 	mem_avoid[2].start = cmd_line;
 	mem_avoid[2].size = cmd_line_size;
+	fill_pagetable(mem_avoid[2].start, mem_avoid[2].size);
 
 	/* Avoid params */
 	mem_avoid[3].start = (unsigned long)real_mode;
 	mem_avoid[3].size = sizeof(*real_mode);
+	fill_pagetable(mem_avoid[3].start, mem_avoid[3].size);
+
+	/* don't need to set mapping for setup_data */
+
+#ifdef CONFIG_X86_VERBOSE_BOOTUP
+	/* for video ram */
+	fill_pagetable(0, PMD_SIZE);
+#endif
 }
 
 /* Does this memory vector overlap a known avoided area? */
@@ -371,6 +382,9 @@ unsigned char *choose_kernel_location(unsigned char *input,
 		goto out;
 
 	choice = random;
+
+	fill_pagetable(choice, output_size);
+	switch_pagetable();
 out:
 	return (unsigned char *)choice;
 }
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 36914515dd76..075bb15f9494 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -126,7 +126,7 @@ ENTRY(startup_32)
 	/* Initialize Page tables to 0 */
 	leal	pgtable(%ebx), %edi
 	xorl	%eax, %eax
-	movl	$((4096*6)/4), %ecx
+	movl	$(BOOT_INIT_PGT_SIZE/4), %ecx
 	rep	stosl
 
 	/* Build Level 4 */
@@ -478,4 +478,4 @@ boot_stack_end:
 	.section ".pgtable","a",@nobits
 	.balign 4096
 pgtable:
-	.fill 6*4096, 1, 0
+	.fill BOOT_PGT_SIZE, 1, 0
diff --git a/arch/x86/boot/compressed/misc.h b/arch/x86/boot/compressed/misc.h
index dcf01c22400e..11736a6a6670 100644
--- a/arch/x86/boot/compressed/misc.h
+++ b/arch/x86/boot/compressed/misc.h
@@ -84,6 +84,17 @@ unsigned char *choose_kernel_location(unsigned char *input,
 }
 #endif
 
+#ifdef CONFIG_X86_64
+void fill_pagetable(unsigned long start, unsigned long size);
+void switch_pagetable(void);
+extern unsigned char _pgtable[];
+#else
+static inline void fill_pagetable(unsigned long start, unsigned long size)
+{ }
+static inline void switch_pagetable(void)
+{ }
+#endif
+
 #ifdef CONFIG_EARLY_PRINTK
 /* early_serial_console.c */
 extern int early_serial_base;
diff --git a/arch/x86/boot/compressed/misc_pgt.c b/arch/x86/boot/compressed/misc_pgt.c
new file mode 100644
index 000000000000..816551d0327c
--- /dev/null
+++ b/arch/x86/boot/compressed/misc_pgt.c
@@ -0,0 +1,93 @@
+#define __pa(x)  ((unsigned long)(x))
+#define __va(x)  ((void *)((unsigned long)(x)))
+
+#include "misc.h"
+
+#include <asm/init.h>
+#include <asm/pgtable.h>
+
+#include "../../mm/ident_map.c"
+#include "../string.h"
+
+struct alloc_pgt_data {
+	unsigned char *pgt_buf;
+	unsigned long pgt_buf_size;
+	unsigned long pgt_buf_offset;
+};
+
+static void *alloc_pgt_page(void *context)
+{
+	struct alloc_pgt_data *d = (struct alloc_pgt_data *)context;
+	unsigned char *p = (unsigned char *)d->pgt_buf;
+
+	if (d->pgt_buf_offset >= d->pgt_buf_size) {
+		debug_putstr("out of pgt_buf in misc.c\n");
+		debug_putaddr(d->pgt_buf_offset);
+		debug_putaddr(d->pgt_buf_size);
+		return NULL;
+	}
+
+	p += d->pgt_buf_offset;
+	d->pgt_buf_offset += PAGE_SIZE;
+
+	return p;
+}
+
+/*
+ * Use a normal definition of memset() from string.c. There are already
+ * included header files which expect a definition of memset() and by
+ * the time we define memset macro, it is too late.
+ */
+#undef memset
+
+unsigned long __force_order;
+static struct alloc_pgt_data pgt_data;
+static struct x86_mapping_info mapping_info;
+static pgd_t *level4p;
+
+void fill_pagetable(unsigned long start, unsigned long size)
+{
+	unsigned long end = start + size;
+
+	if (!level4p) {
+		pgt_data.pgt_buf_offset = 0;
+		mapping_info.alloc_pgt_page = alloc_pgt_page;
+		mapping_info.context = &pgt_data;
+		mapping_info.pmd_flag = __PAGE_KERNEL_LARGE_EXEC;
+
+		/*
+		 * come from startup_32 ?
+		 * then cr3 is _pgtable, we can reuse it.
+		 */
+		level4p = (pgd_t *)read_cr3();
+		if ((unsigned long)level4p == (unsigned long)_pgtable) {
+			pgt_data.pgt_buf = (unsigned char *)_pgtable +
+						 BOOT_INIT_PGT_SIZE;
+			pgt_data.pgt_buf_size = BOOT_PGT_SIZE -
+						 BOOT_INIT_PGT_SIZE;
+			memset((unsigned char *)pgt_data.pgt_buf, 0,
+				pgt_data.pgt_buf_size);
+			debug_putstr("boot via startup_32\n");
+		} else {
+			pgt_data.pgt_buf = (unsigned char *)_pgtable;
+			pgt_data.pgt_buf_size = BOOT_PGT_SIZE;
+			memset((unsigned char *)pgt_data.pgt_buf, 0,
+				pgt_data.pgt_buf_size);
+			debug_putstr("boot via startup_64\n");
+			level4p = (pgd_t *)alloc_pgt_page(&pgt_data);
+		}
+	}
+
+	/* align boundary to 2M */
+	start = round_down(start, PMD_SIZE);
+	end = round_up(end, PMD_SIZE);
+	if (start >= end)
+		return;
+
+	kernel_ident_mapping_init(&mapping_info, level4p, start, end);
+}
+
+void switch_pagetable(void)
+{
+	write_cr3((unsigned long)level4p);
+}
diff --git a/arch/x86/include/asm/boot.h b/arch/x86/include/asm/boot.h
index 6b8d6e8cd449..52a9cbc1419f 100644
--- a/arch/x86/include/asm/boot.h
+++ b/arch/x86/include/asm/boot.h
@@ -32,7 +32,26 @@
 #endif /* !CONFIG_KERNEL_BZIP2 */
 
 #ifdef CONFIG_X86_64
+
 #define BOOT_STACK_SIZE	0x4000
+
+#define BOOT_INIT_PGT_SIZE (6*4096)
+#ifdef CONFIG_RANDOMIZE_BASE
+/*
+ * 1 page for level4, 2 pages for first 2M.
+ * (2+2)*4 pages for kernel, param, cmd_line, random kernel
+ * if all cross 512G boundary.
+ * So total will be 19 pages.
+ */
+#ifdef CONFIG_X86_VERBOSE_BOOTUP
+#define BOOT_PGT_SIZE (19*4096)
+#else
+#define BOOT_PGT_SIZE (17*4096)
+#endif
+#else
+#define BOOT_PGT_SIZE BOOT_INIT_PGT_SIZE
+#endif
+
 #else
 #define BOOT_STACK_SIZE	0x1000
 #endif
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379348 — [PATCH v5 08/21] x86, KASLR: Clean up unused code from old run_size

From: Yinghai Lu <yinghai@kernel.org>

Since run_size is now calculated in misc.c, the old script and associated
argument passing is no longer needed. This patch removes them.

Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Junjie Mao <eternal.n08@gmail.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/Makefile  |  4 +---
 arch/x86/boot/compressed/head_32.S |  3 +--
 arch/x86/boot/compressed/head_64.S |  3 ---
 arch/x86/boot/compressed/misc.c    |  6 ++----
 arch/x86/boot/compressed/mkpiggy.c | 10 ++-------
 arch/x86/tools/calc_run_size.sh    | 42 --------------------------------------
 6 files changed, 6 insertions(+), 62 deletions(-)
 delete mode 100644 arch/x86/tools/calc_run_size.sh

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 323674a9d6db..81a8d0fd34eb 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -109,10 +109,8 @@ suffix-$(CONFIG_KERNEL_XZ)	:= xz
 suffix-$(CONFIG_KERNEL_LZO) 	:= lzo
 suffix-$(CONFIG_KERNEL_LZ4) 	:= lz4
 
-RUN_SIZE = $(shell $(OBJDUMP) -h vmlinux | \
-	     $(CONFIG_SHELL) $(srctree)/arch/x86/tools/calc_run_size.sh)
 quiet_cmd_mkpiggy = MKPIGGY $@
-      cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
+      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
 
 targets += piggy.S
 $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 0c140f99c602..122b32faf3ef 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -210,7 +210,6 @@ relocated:
  * Do the decompression, and jump to the new kernel..
  */
 				/* push arguments for decompress_kernel: */
-	pushl	$z_run_size	/* size of kernel with .bss and .brk */
 	pushl	$z_output_len	/* decompressed length, end of relocs */
 
 	movl    BP_init_size(%esi), %eax
@@ -226,7 +225,7 @@ relocated:
 	pushl	%eax		/* heap area */
 	pushl	%esi		/* real mode pointer */
 	call	decompress_kernel /* returns kernel location in %eax */
-	addl	$28, %esp
+	addl	$24, %esp
 
 /*
  * Jump to the decompressed kernel.
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 67dd8d300c61..36914515dd76 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -407,8 +407,6 @@ relocated:
  * Do the decompression, and jump to the new kernel..
  */
 	pushq	%rsi			/* Save the real mode argument */
-	movq	$z_run_size, %r9	/* size of kernel with .bss and .brk */
-	pushq	%r9
 	movq	%rsi, %rdi		/* real mode address */
 	leaq	boot_heap(%rip), %rsi	/* malloc area for uncompression */
 	leaq	input_data(%rip), %rdx  /* input_data */
@@ -416,7 +414,6 @@ relocated:
 	movq	%rbp, %r8		/* output target address */
 	movq	$z_output_len, %r9	/* decompressed length, end of relocs */
 	call	decompress_kernel	/* returns kernel location in %rax */
-	popq	%r9
 	popq	%rsi
 
 /*
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 4d878a7b9fab..6adb5b827aca 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -393,9 +393,9 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 				  unsigned char *input_data,
 				  unsigned long input_len,
 				  unsigned char *output,
-				  unsigned long output_len,
-				  unsigned long run_size)
+				  unsigned long output_len)
 {
+	const unsigned long run_size = VO__end - VO__text;
 	unsigned char *output_orig = output;
 
 	real_mode = rmode;
@@ -416,8 +416,6 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	lines = real_mode->screen_info.orig_video_lines;
 	cols = real_mode->screen_info.orig_video_cols;
 
-	run_size = VO__end - VO__text;
-
 	console_init();
 	debug_putstr("early console in decompress_kernel\n");
 
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index a613c84d9b88..c51486429bdc 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -34,13 +34,11 @@ int main(int argc, char *argv[])
 {
 	uint32_t olen;
 	long ilen;
-	unsigned long run_size;
 	FILE *f = NULL;
 	int retval = 1;
 
-	if (argc < 3) {
-		fprintf(stderr, "Usage: %s compressed_file run_size\n",
-				argv[0]);
+	if (argc < 2) {
+		fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
 		goto bail;
 	}
 
@@ -65,15 +63,11 @@ int main(int argc, char *argv[])
 	ilen = ftell(f);
 	olen = get_unaligned_le32(&olen);
 
-	run_size = atoi(argv[2]);
-
 	printf(".section \".rodata..compressed\",\"a\",@progbits\n");
 	printf(".globl z_input_len\n");
 	printf("z_input_len = %lu\n", ilen);
 	printf(".globl z_output_len\n");
 	printf("z_output_len = %lu\n", (unsigned long)olen);
-	printf(".globl z_run_size\n");
-	printf("z_run_size = %lu\n", run_size);
 
 	printf(".globl input_data, input_data_end\n");
 	printf("input_data:\n");
diff --git a/arch/x86/tools/calc_run_size.sh b/arch/x86/tools/calc_run_size.sh
deleted file mode 100644
index 1a4c17bb3910..000000000000
--- a/arch/x86/tools/calc_run_size.sh
+++ /dev/null
@@ -1,42 +0,0 @@
-#!/bin/sh
-#
-# Calculate the amount of space needed to run the kernel, including room for
-# the .bss and .brk sections.
-#
-# Usage:
-# objdump -h a.out | sh calc_run_size.sh
-
-NUM='\([0-9a-fA-F]*[ \t]*\)'
-OUT=$(sed -n 's/^[ \t0-9]*.b[sr][sk][ \t]*'"$NUM$NUM$NUM$NUM"'.*/\1\4/p')
-if [ -z "$OUT" ] ; then
-	echo "Never found .bss or .brk file offset" >&2
-	exit 1
-fi
-
-OUT=$(echo ${OUT# })
-sizeA=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-offsetA=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-sizeB=$(printf "%d" 0x${OUT%% *})
-OUT=${OUT#* }
-offsetB=$(printf "%d" 0x${OUT%% *})
-
-run_size=$(( $offsetA + $sizeA + $sizeB ))
-
-# BFD linker shows the same file offset in ELF.
-if [ "$offsetA" -ne "$offsetB" ] ; then
-	# Gold linker shows them as consecutive.
-	endB=$(( $offsetB + $sizeB ))
-	if [ "$endB" != "$run_size" ] ; then
-		printf "sizeA: 0x%x\n" $sizeA >&2
-		printf "offsetA: 0x%x\n" $offsetA >&2
-		printf "sizeB: 0x%x\n" $sizeB >&2
-		printf "offsetB: 0x%x\n" $offsetB >&2
-		echo ".bss and .brk are non-contiguous" >&2
-		exit 1
-	fi
-fi
-
-printf "%d\n" $run_size
-exit 0
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379349 — [PATCH v5 14/21] x86, KASLR: Add slot_area to manage random slots

From: Baoquan He <bhe@redhat.com>

In order to support KASLR moving the kernel anywhere in the whole
of physical memory (could be 64T), we need to handle counting the
potential randomization locations in a more efficient manner.

In the worst case with 64T, there could be roughly 32 * 1024 * 1024
randomization slots if CONFIG_PHYSICAL_ALIGN is 0x1000000. Currently
the starting address of candidate positions is stored into the slots[]
array, one at a time.  With this method will cost too much memory and
it's also very inefficient to get and save the slot information into
slot array one by one.

This patch introduces struct slot_area to manage each contiguous region
of randomization slots. Each slot_area will contain the starting address
and how many available slots are in this area. As with the original code,
the slot_areas will be avoid the mem_avoid regions, since those areas
should be be used for the relocated position.

Since setup_data is a linked list, it could contain an unknown number
of memory regions to be avoided, which could cause us to fragment
the contiguous memory that the slot_area array is tracking. In normal
operation this fragmentation will be extremely unlikely, but choose
a suitably large value (100) for the array. If there are more slots
available beyond 100, they will be ignored for KASLR selection.

Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/aslr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index cd261debead9..abe618d489ea 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -258,8 +258,20 @@ static bool mem_avoid_overlap(struct mem_vector *img)
 }
 
 static unsigned long slots[KERNEL_IMAGE_SIZE / CONFIG_PHYSICAL_ALIGN];
+
+struct slot_area {
+	unsigned long addr;
+	int num;
+};
+
+#define MAX_SLOT_AREA 100
+
+static struct slot_area slot_areas[MAX_SLOT_AREA];
+
 static unsigned long slot_max;
 
+static unsigned long slot_area_index;
+
 static void slots_append(unsigned long addr)
 {
 	/* Overflowing the slots list should be impossible. */
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379350 — [PATCH v5 17/21] x86, KASLR: Clarify purpose of each get_random_long

KASLR will be calling get_random_long twice, but the debug output won't
distinguishing between them. This patch adds a report on when it is
fetching the physical vs virtual address offset. With this, once the
virtual offset is separate, the report changes:

Before:
KASLR using RDTSC...
KASLR using RDTSC...

After:
Physical KASLR using RDTSC...
Virtual KASLR using RDTSC...

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/aslr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index b527ff10372a..c58215d5a80d 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -60,7 +60,7 @@ static unsigned long get_random_boot(void)
 	return hash;
 }
 
-static unsigned long get_random_long(void)
+static unsigned long get_random_long(const char *purpose)
 {
 #ifdef CONFIG_X86_64
 	const unsigned long mix_const = 0x5d6008cbf3848dd3UL;
@@ -70,7 +70,8 @@ static unsigned long get_random_long(void)
 	unsigned long raw, random = get_random_boot();
 	bool use_i8254 = true;
 
-	debug_putstr("KASLR using");
+	debug_putstr(purpose);
+	debug_putstr(" KASLR using");
 
 	if (has_cpuflag(X86_FEATURE_RDRAND)) {
 		debug_putstr(" RDRAND");
@@ -338,7 +339,7 @@ static unsigned long slots_fetch_random(void)
 	if (slot_max == 0)
 		return 0;
 
-	return slots[get_random_long() % slot_max];
+	return slots[get_random_long("Physical") % slot_max];
 }
 
 static void process_e820_entry(struct e820entry *entry,
@@ -425,7 +426,7 @@ static unsigned long find_random_virt_offset(unsigned long minimum,
 	slots = (KERNEL_IMAGE_SIZE - minimum - image_size) /
 		 CONFIG_PHYSICAL_ALIGN + 1;
 
-	random = get_random_long() % slots;
+	random = get_random_long("Virtual") % slots;
 
 	return random * CONFIG_PHYSICAL_ALIGN + minimum;
 }
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379351 — [PATCH v5 11/21] x86, boot: Split out kernel_ident_mapping_init

From: Yinghai Lu <yinghai@kernel.org>

In order to support on-demand page table creation when moving the
kernel for KASLR, we need to use kernel_ident_mapping_init in the
decompression code. This splits it out into its own file for use outside
of init_64.c. Additionally, checking for __pa/__va defines is added
since they need to be overridden in the decompression code.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
[kees: rewrote changelog]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/include/asm/page.h |  5 +++
 arch/x86/mm/ident_map.c     | 74 +++++++++++++++++++++++++++++++++++++++++++++
 arch/x86/mm/init_64.c       | 74 +--------------------------------------------
 3 files changed, 80 insertions(+), 73 deletions(-)
 create mode 100644 arch/x86/mm/ident_map.c

diff --git a/arch/x86/include/asm/page.h b/arch/x86/include/asm/page.h
index 802dde30c928..cf8f619b305f 100644
--- a/arch/x86/include/asm/page.h
+++ b/arch/x86/include/asm/page.h
@@ -37,7 +37,10 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
 	alloc_page_vma(GFP_HIGHUSER | __GFP_ZERO | movableflags, vma, vaddr)
 #define __HAVE_ARCH_ALLOC_ZEROED_USER_HIGHPAGE
 
+#ifndef __pa
 #define __pa(x)		__phys_addr((unsigned long)(x))
+#endif
+
 #define __pa_nodebug(x)	__phys_addr_nodebug((unsigned long)(x))
 /* __pa_symbol should be used for C visible symbols.
    This seems to be the official gcc blessed way to do such arithmetic. */
@@ -51,7 +54,9 @@ static inline void copy_user_page(void *to, void *from, unsigned long vaddr,
 #define __pa_symbol(x) \
 	__phys_addr_symbol(__phys_reloc_hide((unsigned long)(x)))
 
+#ifndef __va
 #define __va(x)			((void *)((unsigned long)(x)+PAGE_OFFSET))
+#endif
 
 #define __boot_va(x)		__va(x)
 #define __boot_pa(x)		__pa(x)
diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
new file mode 100644
index 000000000000..751ca920773a
--- /dev/null
+++ b/arch/x86/mm/ident_map.c
@@ -0,0 +1,74 @@
+
+static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
+			   unsigned long addr, unsigned long end)
+{
+	addr &= PMD_MASK;
+	for (; addr < end; addr += PMD_SIZE) {
+		pmd_t *pmd = pmd_page + pmd_index(addr);
+
+		if (!pmd_present(*pmd))
+			set_pmd(pmd, __pmd(addr | pmd_flag));
+	}
+}
+static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
+			  unsigned long addr, unsigned long end)
+{
+	unsigned long next;
+
+	for (; addr < end; addr = next) {
+		pud_t *pud = pud_page + pud_index(addr);
+		pmd_t *pmd;
+
+		next = (addr & PUD_MASK) + PUD_SIZE;
+		if (next > end)
+			next = end;
+
+		if (pud_present(*pud)) {
+			pmd = pmd_offset(pud, 0);
+			ident_pmd_init(info->pmd_flag, pmd, addr, next);
+			continue;
+		}
+		pmd = (pmd_t *)info->alloc_pgt_page(info->context);
+		if (!pmd)
+			return -ENOMEM;
+		ident_pmd_init(info->pmd_flag, pmd, addr, next);
+		set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
+	}
+
+	return 0;
+}
+
+int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
+			      unsigned long addr, unsigned long end)
+{
+	unsigned long next;
+	int result;
+	int off = info->kernel_mapping ? pgd_index(__PAGE_OFFSET) : 0;
+
+	for (; addr < end; addr = next) {
+		pgd_t *pgd = pgd_page + pgd_index(addr) + off;
+		pud_t *pud;
+
+		next = (addr & PGDIR_MASK) + PGDIR_SIZE;
+		if (next > end)
+			next = end;
+
+		if (pgd_present(*pgd)) {
+			pud = pud_offset(pgd, 0);
+			result = ident_pud_init(info, pud, addr, next);
+			if (result)
+				return result;
+			continue;
+		}
+
+		pud = (pud_t *)info->alloc_pgt_page(info->context);
+		if (!pud)
+			return -ENOMEM;
+		result = ident_pud_init(info, pud, addr, next);
+		if (result)
+			return result;
+		set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
+	}
+
+	return 0;
+}
diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 214afda97911..65cfbeefbec4 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -58,79 +58,7 @@
 
 #include "mm_internal.h"
 
-static void ident_pmd_init(unsigned long pmd_flag, pmd_t *pmd_page,
-			   unsigned long addr, unsigned long end)
-{
-	addr &= PMD_MASK;
-	for (; addr < end; addr += PMD_SIZE) {
-		pmd_t *pmd = pmd_page + pmd_index(addr);
-
-		if (!pmd_present(*pmd))
-			set_pmd(pmd, __pmd(addr | pmd_flag));
-	}
-}
-static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
-			  unsigned long addr, unsigned long end)
-{
-	unsigned long next;
-
-	for (; addr < end; addr = next) {
-		pud_t *pud = pud_page + pud_index(addr);
-		pmd_t *pmd;
-
-		next = (addr & PUD_MASK) + PUD_SIZE;
-		if (next > end)
-			next = end;
-
-		if (pud_present(*pud)) {
-			pmd = pmd_offset(pud, 0);
-			ident_pmd_init(info->pmd_flag, pmd, addr, next);
-			continue;
-		}
-		pmd = (pmd_t *)info->alloc_pgt_page(info->context);
-		if (!pmd)
-			return -ENOMEM;
-		ident_pmd_init(info->pmd_flag, pmd, addr, next);
-		set_pud(pud, __pud(__pa(pmd) | _KERNPG_TABLE));
-	}
-
-	return 0;
-}
-
-int kernel_ident_mapping_init(struct x86_mapping_info *info, pgd_t *pgd_page,
-			      unsigned long addr, unsigned long end)
-{
-	unsigned long next;
-	int result;
-	int off = info->kernel_mapping ? pgd_index(__PAGE_OFFSET) : 0;
-
-	for (; addr < end; addr = next) {
-		pgd_t *pgd = pgd_page + pgd_index(addr) + off;
-		pud_t *pud;
-
-		next = (addr & PGDIR_MASK) + PGDIR_SIZE;
-		if (next > end)
-			next = end;
-
-		if (pgd_present(*pgd)) {
-			pud = pud_offset(pgd, 0);
-			result = ident_pud_init(info, pud, addr, next);
-			if (result)
-				return result;
-			continue;
-		}
-
-		pud = (pud_t *)info->alloc_pgt_page(info->context);
-		if (!pud)
-			return -ENOMEM;
-		result = ident_pud_init(info, pud, addr, next);
-		if (result)
-			return result;
-		set_pgd(pgd, __pgd(__pa(pud) | _KERNPG_TABLE));
-	}
-
-	return 0;
-}
+#include "ident_map.c"
 
 /*
  * NOTE: pagetable_init alloc all the fixmap pagetables contiguous on the
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379352 — [PATCH v5 10/21] x86, KASLR: Consolidate mem_avoid entries

From: Yinghai Lu <yinghai@kernel.org>

The mem_avoid array is used to track positions that should be avoided
(like the compressed kernel, decompression code, etc) when selecting a
memory position for the relocated kernel. Since ZO is now at the end of
the decompression buffer and the decompression code (and its heap and
stack) are at the front, we can safely consolidate the decompression
entry, the heap entry, and the stack entry. The boot_params memory,
however, could be elsewhere, so it should be explicitly included.

Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog, cleaned up code comment]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/aslr.c | 72 ++++++++++++++++++++++++++++++++---------
 1 file changed, 56 insertions(+), 16 deletions(-)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 462654097d63..277c7a4ec3a3 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -109,7 +109,7 @@ struct mem_vector {
 	unsigned long size;
 };
 
-#define MEM_AVOID_MAX 5
+#define MEM_AVOID_MAX 4
 static struct mem_vector mem_avoid[MEM_AVOID_MAX];
 
 static bool mem_contains(struct mem_vector *region, struct mem_vector *item)
@@ -134,22 +134,66 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
 	return true;
 }
 
+/*
+ * In theroy, KASLR can put the kernel anywhere in area of [16M, 64T). The
+ * mem_avoid array is used to store the ranges that need be avoided when
+ * KASLR searches for the relocated address. We must avoid any regions that
+ * are unsafe to overlap with during decompression, and other things like
+ * the initrd, cmdline and boot_params.
+ *
+ * How to calculate the unsafe area used by decompression is detailed here.
+ * The compressed vmlinux (ZO) plus relocs and the run space of ZO can't be
+ * overwritten by decompression output.
+ *
+ * ZO sits against the end of the decompression buffer, so we can calculate
+ * where text, data, bss, etc of ZO are positioned.
+ *
+ * The follow are already enforced by the code:
+ *  - init_size >= run_size,
+ *  - input+input_len >= output+output_len,
+ *  - run_size could be >= or < output_len
+ *
+ * From this, we can make several observations, illustrated by a diagram:
+ *  - init_size > run_size
+ *  - input+input_len > output+output_len
+ *  - run_size > output_len
+ *
+ * 0   output            input            input+input_len    output+init_size
+ * |     |                 |                       |                       |
+ * |-----|--------|--------|------------------|----|------------|----------|
+ *                |                           |                 |
+ *   output+init_size-ZO_INIT_SIZE    output+output_len    output+run_size
+ *
+ * [output, output+init_size) is for the buffer for decompressing the
+ * compressed kernel (ZO).
+ *
+ * [output, output+run_size) is for the uncompressed kernel (VO) run size.
+ * [output, output+output_len) is VO plus relocs
+ *
+ * [output+init_size-ZO_INIT_SIZE, output+init_size) is copied ZO.
+ * [input, input+input_len) is copied compressed (VO (vmlinux after objcopy)
+ * plus relocs), not the ZO.
+ *
+ * [input+input_len, output+init_size) is [_text, _end) for ZO. That was the
+ * first range in mem_avoid, which included ZO's heap and stack. Also
+ * [input, input+input_size) need be put in mem_avoid array, but since it
+ * is adjacent to the first entry, they can be merged. This is how we get
+ * the first entry in mem_avoid[].
+ */
 static void mem_avoid_init(unsigned long input, unsigned long input_size,
-			   unsigned long output, unsigned long output_size)
+			   unsigned long output)
 {
+	unsigned long init_size = real_mode->hdr.init_size;
 	u64 initrd_start, initrd_size;
 	u64 cmd_line, cmd_line_size;
-	unsigned long unsafe, unsafe_len;
 	char *ptr;
 
 	/*
 	 * Avoid the region that is unsafe to overlap during
-	 * decompression (see calculations at top of misc.c).
+	 * decompression.
 	 */
-	unsafe_len = (output_size >> 12) + 32768 + 18;
-	unsafe = (unsigned long)input + input_size - unsafe_len;
-	mem_avoid[0].start = unsafe;
-	mem_avoid[0].size = unsafe_len;
+	mem_avoid[0].start = input;
+	mem_avoid[0].size = (output + init_size) - input;
 
 	/* Avoid initrd. */
 	initrd_start  = (u64)real_mode->ext_ramdisk_image << 32;
@@ -169,13 +213,9 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 	mem_avoid[2].start = cmd_line;
 	mem_avoid[2].size = cmd_line_size;
 
-	/* Avoid heap memory. */
-	mem_avoid[3].start = (unsigned long)free_mem_ptr;
-	mem_avoid[3].size = BOOT_HEAP_SIZE;
-
-	/* Avoid stack memory. */
-	mem_avoid[4].start = (unsigned long)free_mem_end_ptr;
-	mem_avoid[4].size = BOOT_STACK_SIZE;
+	/* Avoid params */
+	mem_avoid[3].start = (unsigned long)real_mode;
+	mem_avoid[3].size = sizeof(*real_mode);
 }
 
 /* Does this memory vector overlap a known avoided area? */
@@ -317,7 +357,7 @@ unsigned char *choose_kernel_location(unsigned char *input,
 
 	/* Record the various known unsafe memory ranges. */
 	mem_avoid_init((unsigned long)input, input_size,
-		       (unsigned long)output, output_size);
+		       (unsigned long)output);
 
 	/* Walk e820 and find a random address. */
 	random = find_random_addr(choice, output_size);
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379353 — [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET

From: Baoquan He <bhe@redhat.com>

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 <bhe@redhat.com>
[kees: rewrote changelog, dropped KERNEL_IMAGE_SIZE_DEFAULT, moved earlier]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 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
+	   attempts relying on knowledge of the location of kernel
+	   internals.
+
+	   The kernel physical address can be randomized from 16M to
+	   64T at most. 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.
 
 	   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.
 
-	   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.
 
 	   If unsure, say N.
 
-config RANDOMIZE_BASE_MAX_OFFSET
-	hex "Maximum kASLR offset allowed" if EXPERT
-	depends on RANDOMIZE_BASE
-	range 0x0 0x20000000 if X86_32
-	default "0x20000000" if X86_32
-	range 0x0 0x40000000 if X86_64
-	default "0x40000000" if X86_64
-	---help---
-	  The lesser of RANDOMIZE_BASE_MAX_OFFSET and available physical
-	  memory is used to determine the maximal offset in bytes that will
-	  be applied to the kernel when kernel Address Space Layout
-	  Randomization (kASLR) is active. This must be a multiple of
-	  PHYSICAL_ALIGN.
-
-	  On 32-bit this is limited to 512MiB by page table layouts. The
-	  default is 512MiB.
-
-	  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_MAX_OFFSET is above 512MiB, the
-	  modules area will shrink to compensate, up to the current maximum
-	  1GiB to 1GiB split. The default is 1GiB.
-
-	  If unsure, leave at the default value.
-
 # Relocation on x86 needs some additional build support
 config X86_NEED_RELOCS
 	def_bool y
diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index 622aa881c6ab..462654097d63 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -206,15 +206,13 @@ static bool mem_avoid_overlap(struct mem_vector *img)
 	return false;
 }
 
-static unsigned long slots[CONFIG_RANDOMIZE_BASE_MAX_OFFSET /
-			   CONFIG_PHYSICAL_ALIGN];
+static unsigned long slots[KERNEL_IMAGE_SIZE / CONFIG_PHYSICAL_ALIGN];
 static unsigned long slot_max;
 
 static void slots_append(unsigned long addr)
 {
 	/* Overflowing the slots list should be impossible. */
-	if (slot_max >= CONFIG_RANDOMIZE_BASE_MAX_OFFSET /
-			CONFIG_PHYSICAL_ALIGN)
+	if (slot_max >= KERNEL_IMAGE_SIZE / CONFIG_PHYSICAL_ALIGN)
 		return;
 
 	slots[slot_max++] = addr;
@@ -240,7 +238,7 @@ static void process_e820_entry(struct e820entry *entry,
 		return;
 
 	/* Ignore entries entirely above our maximum. */
-	if (entry->addr >= CONFIG_RANDOMIZE_BASE_MAX_OFFSET)
+	if (entry->addr >= KERNEL_IMAGE_SIZE)
 		return;
 
 	/* Ignore entries entirely below our minimum. */
@@ -265,8 +263,8 @@ static void process_e820_entry(struct e820entry *entry,
 	region.size -= region.start - entry->addr;
 
 	/* Reduce maximum size to fit end of image within maximum limit. */
-	if (region.start + region.size > CONFIG_RANDOMIZE_BASE_MAX_OFFSET)
-		region.size = CONFIG_RANDOMIZE_BASE_MAX_OFFSET - region.start;
+	if (region.start + region.size > KERNEL_IMAGE_SIZE)
+		region.size = KERNEL_IMAGE_SIZE - region.start;
 
 	/* Walk each aligned slot and check for avoided areas. */
 	for (img.start = region.start, img.size = image_size ;
diff --git a/arch/x86/include/asm/page_64_types.h b/arch/x86/include/asm/page_64_types.h
index 4928cf0d5af0..d5c2f8b40faa 100644
--- a/arch/x86/include/asm/page_64_types.h
+++ b/arch/x86/include/asm/page_64_types.h
@@ -47,12 +47,10 @@
  * are fully set up. If kernel ASLR is configured, it can extend the
  * kernel page table mapping, reducing the size of the modules area.
  */
-#define KERNEL_IMAGE_SIZE_DEFAULT      (512 * 1024 * 1024)
-#if defined(CONFIG_RANDOMIZE_BASE) && \
-	CONFIG_RANDOMIZE_BASE_MAX_OFFSET > KERNEL_IMAGE_SIZE_DEFAULT
-#define KERNEL_IMAGE_SIZE   CONFIG_RANDOMIZE_BASE_MAX_OFFSET
+#if defined(CONFIG_RANDOMIZE_BASE)
+#define KERNEL_IMAGE_SIZE	(1024 * 1024 * 1024)
 #else
-#define KERNEL_IMAGE_SIZE      KERNEL_IMAGE_SIZE_DEFAULT
+#define KERNEL_IMAGE_SIZE	(512 * 1024 * 1024)
 #endif
 
 #endif /* _ASM_X86_PAGE_64_DEFS_H */
diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
index bd7a9b9e2e14..f2ee42d61894 100644
--- a/arch/x86/mm/init_32.c
+++ b/arch/x86/mm/init_32.c
@@ -804,9 +804,6 @@ void __init mem_init(void)
 	BUILD_BUG_ON(VMALLOC_START			>= VMALLOC_END);
 #undef high_memory
 #undef __FIXADDR_TOP
-#ifdef CONFIG_RANDOMIZE_BASE
-	BUILD_BUG_ON(CONFIG_RANDOMIZE_BASE_MAX_OFFSET > KERNEL_IMAGE_SIZE);
-#endif
 
 #ifdef CONFIG_HIGHMEM
 	BUG_ON(PKMAP_BASE + LAST_PKMAP*PAGE_SIZE	> FIXADDR_START);
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379528 — Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET

* Kees Cook <keescook@chromium.org> wrote:

> From: Baoquan He <bhe@redhat.com>
> 
> 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 <bhe@redhat.com>
> [kees: rewrote changelog, dropped KERNEL_IMAGE_SIZE_DEFAULT, moved earlier]
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
>  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

[toc] | [prev] | [next] | [standalone]

#1380142 — Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET

On Fri, Apr 15, 2016 at 1:07 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> From: Baoquan He <bhe@redhat.com>
>>
>> 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 <bhe@redhat.com>
>> [kees: rewrote changelog, dropped KERNEL_IMAGE_SIZE_DEFAULT, moved earlier]
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>>  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

Gah, sorry. I was reading these, but things slip by. I'll fix it. (And
add these to the common misspellings that checkpatch.pl looks for.)

>
>> +        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.

Yup, good idea.

> 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.)

This will need some thought... is it better to fail to boot or to boot
without entropy? As-is, it's possible to randomly position the kernel
base address at exactly the location it was going to boot without
KASLR too, yet this is still considered random...

>
>
>>                           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.

Trying to capture the effect of reducing the kernel/module split
without detailing the actual numbers may sound evasive, but I'll see
what I can do.

>
>>          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?

IIRC, hpa explicitly did not want to do this when he was making
suggestions on this area. I would need to dig out the thread -- I
can't find it now. I'd prefer to leave this as-is, since changing it
would add yet another variable to the behavioral changes of this
series.

>> -        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.

Gah, I need to teach my spell checker about #defines. ;) I read this
multiple times after you called it out and still couldn't see it
(http://www.mrc-cbu.cam.ac.uk/people/matt.davis/cmabridge/). Finally
dropped the _ and the spell checker flagged it. ;)

> 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.

Totally fine by me. I prefer MB/GB. I wonder if it is worth
documenting this preference somewhere in the style guide? It's
certainly rare in the kernel, but it's present (and there are even
#defines for it *sob*).

$ git grep '[KMGTP]iB' | wc -l
1239
$ git grep '[KMGTP]B' | wc -l
192251

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1380511 — Re: [PATCH v5 03/21] x86, KASLR: Drop CONFIG_RANDOMIZE_BASE_MAX_OFFSET

* Kees Cook <keescook@chromium.org> wrote:

> > 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.)
> 
> This will need some thought... is it better to fail to boot or to boot without 
> entropy? As-is, it's possible to randomly position the kernel base address at 
> exactly the location it was going to boot without KASLR too, yet this is still 
> considered random...

I think we should boot but print a prominent warning. On real systems it's not 
supposed to happen, right? So we want to know if it happens, but don't want to 
hassle the user with breaking the system by not booting.

> > Also, could we always mix the i8254 timer into this as well, not just when 
> > RDTSC is unavailable?
> 
> IIRC, hpa explicitly did not want to do this when he was making
> suggestions on this area. I would need to dig out the thread -- I
> can't find it now. I'd prefer to leave this as-is, since changing it
> would add yet another variable to the behavioral changes of this
> series.

Sure, can stay as-is for now.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1379354 — [PATCH v5 06/21] x86, KASLR: Update description for decompressor worst case size

From: Baoquan He <bhe@redhat.com>

The comment that describes the analysis for the size of the decompressor
code only took gzip into account (there are 6 other decompressors that
could be used). The actual z_extract_offset calculation in code was
already handling the correct maximum size, but this documentation hadn't
been updated. This updates the documentation and fixes several typos.

Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog, cleaned up comment style]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/compressed/misc.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index e2a998f8c304..31e2d6155643 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -19,11 +19,13 @@
  */
 
 /*
- * Getting to provable safe in place decompression is hard.
- * Worst case behaviours need to be analyzed.
- * Background information:
+ * Getting to provable safe in place decompression is hard. Worst case
+ * behaviours need be analyzed. Here let's take decompressing gzip-compressed
+ * kernel as example to illustrate it.
+ *
+ * The file layout of gzip compressed kernel is as follows. For more
+ * information, please refer to RFC 1951 and RFC 1952.
  *
- * The file layout is:
  *    magic[2]
  *    method[1]
  *    flags[1]
@@ -70,13 +72,13 @@
  * of 5 bytes per 32767 bytes.
  *
  * The worst case internal to a compressed block is very hard to figure.
- * The worst case can at least be boundined by having one bit that represents
+ * The worst case can at least be bounded by having one bit that represents
  * 32764 bytes and then all of the rest of the bytes representing the very
  * very last byte.
  *
  * All of which is enough to compute an amount of extra data that is required
  * to be safe.  To avoid problems at the block level allocating 5 extra bytes
- * per 32767 bytes of data is sufficient.  To avoind problems internal to a
+ * per 32767 bytes of data is sufficient.  To avoid problems internal to a
  * block adding an extra 32767 bytes (the worst case uncompressed block size)
  * is sufficient, to ensure that in the worst case the decompressed data for
  * block will stop the byte before the compressed data for a block begins.
@@ -88,11 +90,17 @@
  * Adding 8 bytes per 32K is a bit excessive but much easier to calculate.
  * Adding 32768 instead of 32767 just makes for round numbers.
  *
+ * Above analysis is for decompressing gzip compressed kernel only. Up to
+ * now 6 different decompressor are supported all together. And among them
+ * xz stores data in chunks and has maximum chunk of 64K. Hence safety
+ * margin should be updated to cover all decompressors so that we don't
+ * need to deal with each of them separately. Please check
+ * the description in lib/decompressor_xxx.c for specific information.
+ *
+ * extra_bytes = (uncompressed_size >> 12) + 65536 + 128.
+ *
  */
 
-/*
- * gzip declarations
- */
 #define STATIC		static
 
 #undef memcpy
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379977 — Re: [PATCH v5 06/21] x86, KASLR: Update description for decompressor worst case size

On 2016-04-14 Kees Cook wrote:
> + * Above analysis is for decompressing gzip compressed kernel only. Up to
> + * now 6 different decompressor are supported all together.

There are six decompressors in Linux now, but the number can change and
the comment become outdated, so I suggest omitting the exact number here.

> And among them
> + * xz stores data in chunks and has maximum chunk of 64K. Hence safety
> + * margin should be updated to cover all decompressors so that we don't
> + * need to deal with each of them separately. Please check
> + * the description in lib/decompressor_xxx.c for specific information.
> + *
> + * extra_bytes = (uncompressed_size >> 12) + 65536 + 128.

lib/decompress_xxx.c, not lib/decompressor_xxx.c.

Referring to those files is problematic because only decompress_unxz.c
talks about the safety margin. The explanation of the safety margin for
gzip is still in misc.c instead of decompress_inflate.c.

I suspect that safety margin calculations haven't been made for other
compressors in Linux, so there is nothing that people could read for
more information. At least such information isn't present in the
comments or commit messages.

For example (and a bit off-topic), there is a possible sign of too small
safety margin in decompress_unlzo.c, where a memcpy call can get
overlapping dest and src buffers:

    /* When the input data is not compressed at all,
     * lzo1x_decompress_safe will fail, so call memcpy()
     * instead */
    if (unlikely(dst_len == src_len))
            memcpy(out_buf, in_buf, src_len);

The overlap can only happen if there's enough incompressible data near
the end of the kernel image. It still works in practice as long as
memcpy works with overlapping buffers for dest < src.

-- 
Lasse Collin  |  IRC: Larhzu @ IRCnet & Freenode

[toc] | [prev] | [next] | [standalone]

#1379355 — [PATCH v5 07/21] x86, boot: Fix run_size calculation

From: Yinghai Lu <yinghai@kernel.org>

Currently, the kernel run_size (size of code plus brk and bss) is
calculated via the shell script arch/x86/tools/calc_run_size.sh. It gets
the file offset and mem size of for the .bss and .brk sections in vmlinux,
and adds them as follows:

run_size=$(( $offsetA + $sizeA + $sizeB ))

However, this is not correct (it is too large). To illustrate, here's
a walk-through of the script's calculation, compared to the correct way
to find it.

First, offsetA is found as the starting address of the first .bss or
.brk section seen in the ELF file. The sizeA and sizeB values are the
respective section sizes.

[bhe@x1 linux]$ objdump -h vmlinux

vmlinux:     file format elf64-x86-64

Sections:
Idx Name    Size      VMA               LMA               File off  Algn
 27 .bss    00170000  ffffffff81ec8000  0000000001ec8000  012c8000  2**12
            ALLOC
 28 .brk    00027000  ffffffff82038000  0000000002038000  012c8000  2**0
            ALLOC

Here, offsetA is 0x012c8000, with sizeA at 0x00170000 and sizeB at
0x00027000. The resulting run_size is 0x145f000:

0x012c8000 + 0x00170000 + 0x00027000 = 0x145f000

However, if we instead examine the ELF LOAD program headers, we see a
different picture.

[bhe@x1 linux]$ readelf -l vmlinux

Elf file type is EXEC (Executable file)
Entry point 0x1000000
There are 5 program headers, starting at offset 64

Program Headers:
  Type        Offset             VirtAddr           PhysAddr
              FileSiz            MemSiz              Flags  Align
  LOAD        0x0000000000200000 0xffffffff81000000 0x0000000001000000
              0x0000000000b5e000 0x0000000000b5e000  R E    200000
  LOAD        0x0000000000e00000 0xffffffff81c00000 0x0000000001c00000
              0x0000000000145000 0x0000000000145000  RW     200000
  LOAD        0x0000000001000000 0x0000000000000000 0x0000000001d45000
              0x0000000000018158 0x0000000000018158  RW     200000
  LOAD        0x000000000115e000 0xffffffff81d5e000 0x0000000001d5e000
              0x000000000016a000 0x0000000000301000  RWE    200000
  NOTE        0x000000000099bcac 0xffffffff8179bcac 0x000000000179bcac
              0x00000000000001bc 0x00000000000001bc         4

 Section to Segment mapping:
  Segment Sections...
   00     .text .notes __ex_table .rodata __bug_table .pci_fixup .tracedata
          __ksymtab __ksymtab_gpl __ksymtab_strings __init_rodata __param
          __modver
   01     .data .vvar
   02     .data..percpu
   03     .init.text .init.data .x86_cpu_dev.init .parainstructions
          .altinstructions .altinstr_replacement .iommu_table .apicdrivers
          .exit.text .smp_locks .bss .brk
   04     .notes

As mentioned, run_size needs to be the size of the running kernel
including .bss and .brk. We can see from the Section/Segment mapping
above that .bss and .brk are included in segment 03 (which corresponds
to the final LOAD program header). To find the run_size, we calculate
the end of the LOAD segment from its PhysAddr start (0x0000000001d5e000)
and its MemSiz (0x0000000000301000), minus the physical load address of
the kernel (the first LOAD segment's PhysAddr: 0x0000000001000000). The
resulting run_size is 0x105f000:

0x0000000001d5e000 + 0x0000000000301000 - 0x0000000001000000 = 0x105f000

So, from this we can see that the existing run_size calculation is
0x400000 too high. And, as it turns out, the correct run_size is
actually equal to VO_end - VO_text, which is certainly easier to calculate.
_end: 0xffffffff8205f000
_text:0xffffffff81000000

0xffffffff8205f000 - 0xffffffff81000000 = 0x105f000

As a result, run_size is a simple constant, so we don't need to pass it
around; we already have voffset.h such things. We can share voffset.h
between misc.c and header.S instead of getting run_size in other ways.
This patch moves voffset.h creation code to boot/compressed/Makefile,
and switches misc.c to use the VO_end - VO_text calculation.

Dependence before:
boot/header.S ==> boot/voffset.h ==> vmlinux
boot/header.S ==> compressed/vmlinux ==> compressed/misc.c

Dependence after:
boot/header.S ==> compressed/vmlinux ==> compressed/misc.c ==>
                  boot/voffset.h ==> vmlinux

Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
Cc: Junjie Mao <eternal.n08@gmail.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Baoquan He <bhe@redhat.com>
[kees: rewrote changelog]
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/boot/Makefile            | 11 +----------
 arch/x86/boot/compressed/Makefile | 12 ++++++++++++
 arch/x86/boot/compressed/misc.c   |  3 +++
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 942f7dabfb1e..700a9c6e6159 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -86,15 +86,6 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE
 
 SETUP_OBJS = $(addprefix $(obj)/,$(setup-y))
 
-sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(_text\|_end\)$$/\#define VO_\2 0x\1/p'
-
-quiet_cmd_voffset = VOFFSET $@
-      cmd_voffset = $(NM) $< | sed -n $(sed-voffset) > $@
-
-targets += voffset.h
-$(obj)/voffset.h: vmlinux FORCE
-	$(call if_changed,voffset)
-
 sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p'
 
 quiet_cmd_zoffset = ZOFFSET $@
@@ -106,7 +97,7 @@ $(obj)/zoffset.h: $(obj)/compressed/vmlinux FORCE
 
 
 AFLAGS_header.o += -I$(obj)
-$(obj)/header.o: $(obj)/voffset.h $(obj)/zoffset.h
+$(obj)/header.o: $(obj)/zoffset.h
 
 LDFLAGS_setup.elf	:= -T
 $(obj)/setup.elf: $(src)/setup.ld $(SETUP_OBJS) FORCE
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 6915ff2bd996..323674a9d6db 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -45,6 +45,18 @@ LDFLAGS_vmlinux := -T
 hostprogs-y	:= mkpiggy
 HOST_EXTRACFLAGS += -I$(srctree)/tools/include
 
+sed-voffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(_text\|_end\)$$/\#define VO_\2 _AC(0x\1,UL)/p'
+
+quiet_cmd_voffset = VOFFSET $@
+      cmd_voffset = $(NM) $< | sed -n $(sed-voffset) > $@
+
+targets += ../voffset.h
+
+$(obj)/../voffset.h: vmlinux FORCE
+	$(call if_changed,voffset)
+
+$(obj)/misc.o: $(obj)/../voffset.h
+
 vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/head_$(BITS).o $(obj)/misc.o \
 	$(obj)/string.o $(obj)/cmdline.o \
 	$(obj)/piggy.o $(obj)/cpuflags.o
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 31e2d6155643..4d878a7b9fab 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -11,6 +11,7 @@
 
 #include "misc.h"
 #include "../string.h"
+#include "../voffset.h"
 
 /* WARNING!!
  * This code is compiled with -fPIC and it is relocated dynamically
@@ -415,6 +416,8 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	lines = real_mode->screen_info.orig_video_lines;
 	cols = real_mode->screen_info.orig_video_cols;
 
+	run_size = VO__end - VO__text;
+
 	console_init();
 	debug_putstr("early console in decompress_kernel\n");
 
-- 
2.6.3

[toc] | [prev] | [next] | [standalone]

#1379550 — Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation

* Kees Cook <keescook@chromium.org> wrote:

> From: Yinghai Lu <yinghai@kernel.org>
> 
> Currently, the kernel run_size (size of code plus brk and bss) is
> calculated via the shell script arch/x86/tools/calc_run_size.sh.
> It gets the file offset and mem size of for the .bss and .brk sections in 
> vmlinux, and adds them as follows:

'of for'?

> So, from this we can see that the existing run_size calculation is
> 0x400000 too high. 

Btw., why is it too high? Were some sections discarded?

> [...] And, as it turns out, the correct run_size is
> actually equal to VO_end - VO_text, which is certainly easier to calculate.
> _end: 0xffffffff8205f000
> _text:0xffffffff81000000
> 
> 0xffffffff8205f000 - 0xffffffff81000000 = 0x105f000
> 
> As a result, run_size is a simple constant, so we don't need to pass it
> around; we already have voffset.h such things.

(Typo.)

> [...] We can share voffset.h
> between misc.c and header.S instead of getting run_size in other ways.
> This patch moves voffset.h creation code to boot/compressed/Makefile,
> and switches misc.c to use the VO_end - VO_text calculation.

Btw., 'run_size' is a pretty meaningless name, it doesn't really tell us the most 
important thing: that this is the complete size of the uncompressed kernel, the 
total physically continuous size we need to allocate to it so it can execute?

So can we rename it to something more expressive, such as kernel_total_size or so?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1380167 — Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation

On Fri, Apr 15, 2016 at 1:31 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Kees Cook <keescook@chromium.org> wrote:
>
>> From: Yinghai Lu <yinghai@kernel.org>
>>
>> Currently, the kernel run_size (size of code plus brk and bss) is
>> calculated via the shell script arch/x86/tools/calc_run_size.sh.
>> It gets the file offset and mem size of for the .bss and .brk sections in
>> vmlinux, and adds them as follows:
>
> 'of for'?
>
>> So, from this we can see that the existing run_size calculation is
>> 0x400000 too high.
>
> Btw., why is it too high? Were some sections discarded?

I'm not sure why it's that specific value, but the old method used the
section's ELF _file_ position, not the virtual address, and that
seemed to cause the problem.

>
>> [...] And, as it turns out, the correct run_size is
>> actually equal to VO_end - VO_text, which is certainly easier to calculate.
>> _end: 0xffffffff8205f000
>> _text:0xffffffff81000000
>>
>> 0xffffffff8205f000 - 0xffffffff81000000 = 0x105f000
>>
>> As a result, run_size is a simple constant, so we don't need to pass it
>> around; we already have voffset.h such things.
>
> (Typo.)
>
>> [...] We can share voffset.h
>> between misc.c and header.S instead of getting run_size in other ways.
>> This patch moves voffset.h creation code to boot/compressed/Makefile,
>> and switches misc.c to use the VO_end - VO_text calculation.
>
> Btw., 'run_size' is a pretty meaningless name, it doesn't really tell us the most
> important thing: that this is the complete size of the uncompressed kernel, the
> total physically continuous size we need to allocate to it so it can execute?
>
> So can we rename it to something more expressive, such as kernel_total_size or so?

You got it. Thanks again for digging through all this!

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security

[toc] | [prev] | [next] | [standalone]

#1380513 — Re: [PATCH v5 07/21] x86, boot: Fix run_size calculation

* Kees Cook <keescook@chromium.org> wrote:

> > So can we rename it to something more expressive, such as kernel_total_size or 
> > so?
> 
> You got it. Thanks again for digging through all this!

You are welcome! A couple of logistical suggestions:

Could you please split up the series a bit and limit the next series to say no 
more than around 5 patches? (Can be a little bit more when justified to finish up 
a particular line of thought) That way I can apply them in reviewable groups, 
without having to reject the whole series because some patch deep into the series 
has some problem.

I'd suggest starting with absolutely critical fixes (if any!) as-is, to make 
backporting easier. By the looks of it I don't think there's any such patch in 
this series, but just in case there are any, they can be at the front.

Then come the various cleanup patches and non-critical fixes - everything that is 
not supposed to change the behavior of the kernel. I'd suggest doing them in 
roughly this order:

 - file renames first - so that any later revert in a smaller patch does not have
   to go through a rename barrier.

 - then .o-invariant trivial cleanups, the fixing, harmonization (and creation ;-)
   of comments.

 - then come more involved cleanups like moving logic from build time to boot
   time, stricter bounds checks, non-essential fixes, etc.

It might be useful if you declared at this stage that you are mostly done with the 
preparatory work and that the code base is ready for heavier changes, so that 
people (and me) can review the whole source for anything missing. Often a car 
needs a good power wash before we can tell what body work is needed.

... and once we are happy and proud about the code base, then come the more 
exciting things: more fundamental changes, and new features - on top of a squeaky 
clean code base.

This all can happen pretty quickly, as long as the ordering is proper.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel

csiph-web