LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH v4] x86, boot: Make data from decompress_kernel stage live longer
@ 2015-03-15  7:49 Yinghai Lu
  2015-03-16  7:45 ` Ingo Molnar
  0 siblings, 1 reply; 3+ messages in thread
From: Yinghai Lu @ 2015-03-15  7:49 UTC (permalink / raw)
  To: H. Peter Anvin, Ingo Molnar, Kees Cook, Borislav Petkov, Baoquan He
  Cc: Thomas Gleixner, Jiri Kosina, Andrew Morton, Linus Torvalds,
	linux-kernel, Yinghai Lu, Matt Fleming

After commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address
calculation"), get warning about setup_data from debugfs for setup_data.
Boris found setup_data for kaslr from boot stage become all 0's in kernel stage.

Current there is overlapping between data section for decompress code and final
kernel running code.

We need to avoid overlapping to make data live longer and to get the passed
properly.

Current code is using extract_offset to control copied kernel position, it
will put the copied kernel in the middle of buffer when kernel run size is
bigger then decompressed needed buffer size. That cause the overlapping.

Detail flow in current code:
Bootloader allocate buffer according to init_size in hdr, and load the
ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer.
During running of ZO, ZO move itself to the middle of buffer at
z_extract_offset to make sure that decompressor would not have output
overwrite input data before input data get consumed.
After decompressor is done, VO use most part buffer from start.
and ZO code and data section will overlap with VO bss section.
And later VO/clear_bss() clear them before code in arch/x86/kernel/setup.c
try to access them.
Current layout:
when init_size is the same as kernel run_size:
                                        run_size
0              extract_offset          init_size
|------------------|------------------------|
   VO text/data                   VO bss/brk
                   input ZO text ZO data

This patch try to:
At first, move ZO to the end of buffer instead of middle of the buffer.
When init_size is bigger than kernel run size.  like

0                            run_size    init_size
|--------------------------------|----------|
   VO text/data        VO bss/brk
                       input ZO text ZO data

We already have init_size the buffer size, we can find the end easily.

Secondly, add extra size (ZO data size) to init_size. That is for
even old init_size is same as kernel run size.

                                         run_size
0                                   old init_size init_size
|------------------------------------------|--------|
   VO text/data                  VO bss/brk
                               input ZO text ZO data

Here the size changes when old init_size is same as kernel run_size.
# size arch/x86/boot/compressed/vmlinux
   text	   data	    bss	    dec	    hex	filename
13247288    264	  49248	13296800 cae4a0	arch/x86/boot/compressed/vmlinux
# bootloader reported init_size
kernel: [13cc00000, 13ff8efff]

After patch:
#size arch/x86/boot/compressed/vmlinux
   text	   data	    bss	    dec	    hex	filename
13247289    264	  49248	13296801 cae4a1	arch/x86/boot/compressed/vmlinux
# bootloader reported init_size
kernel: [13cc00000, 13ffa2fff]

so init_size increase 20 pages 80k.

Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
-v2: increase init_size in arch/x86/boot/header.S instead of BRK.
     so we can avoid guessing decompressor size.
-v4: add extra checking when ZO_INIT_SIZE>VO_INIT_SIZE, only need
     add the difference to cover ADD_ZO

---
 arch/x86/boot/Makefile                 |    2 +-
 arch/x86/boot/compressed/head_32.S     |   11 +++++++++--
 arch/x86/boot/compressed/head_64.S     |    8 ++++++--
 arch/x86/boot/compressed/mkpiggy.c     |    7 ++-----
 arch/x86/boot/compressed/vmlinux.lds.S |    2 ++
 arch/x86/boot/header.S                 |   14 ++++++++++++--
 arch/x86/kernel/asm-offsets.c          |    1 +
 arch/x86/kernel/vmlinux.lds.S          |    1 +
 8 files changed, 34 insertions(+), 12 deletions(-)

Index: linux-2.6/arch/x86/boot/compressed/head_64.S
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/head_64.S
+++ linux-2.6/arch/x86/boot/compressed/head_64.S
@@ -101,7 +101,9 @@ ENTRY(startup_32)
 1:
 
 	/* Target address to relocate to for decompression */
-	addl	$z_extract_offset, %ebx
+	movl	BP_init_size(%esi), %eax
+	subl	$_end, %eax
+	addl	%eax, %ebx
 
 /*
  * Prepare for entering 64 bit mode
@@ -329,7 +331,9 @@ preferred_addr:
 1:
 
 	/* Target address to relocate to for decompression */
-	leaq	z_extract_offset(%rbp), %rbx
+	movl	BP_init_size(%rsi), %ebx
+	subl	$_end, %ebx
+	addq	%rbp, %rbx
 
 	/* Set up the stack */
 	leaq	boot_stack_end(%rbx), %rsp
Index: linux-2.6/arch/x86/kernel/asm-offsets.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/asm-offsets.c
+++ linux-2.6/arch/x86/kernel/asm-offsets.c
@@ -66,6 +66,7 @@ void common(void) {
 	OFFSET(BP_hardware_subarch, boot_params, hdr.hardware_subarch);
 	OFFSET(BP_version, boot_params, hdr.version);
 	OFFSET(BP_kernel_alignment, boot_params, hdr.kernel_alignment);
+	OFFSET(BP_init_size, boot_params, hdr.init_size);
 	OFFSET(BP_pref_address, boot_params, hdr.pref_address);
 	OFFSET(BP_code32_start, boot_params, hdr.code32_start);
 
Index: linux-2.6/arch/x86/boot/compressed/head_32.S
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/head_32.S
+++ linux-2.6/arch/x86/boot/compressed/head_32.S
@@ -147,7 +147,9 @@ preferred_addr:
 1:
 
 	/* Target address to relocate to for decompression */
-	addl	$z_extract_offset, %ebx
+	movl    BP_init_size(%esi), %eax
+	subl    $_end, %eax
+	addl    %eax, %ebx
 
 	/* Set up the stack */
 	leal	boot_stack_end(%ebx), %esp
@@ -209,8 +211,13 @@ relocated:
 				/* 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 */
-	leal	z_extract_offset_negative(%ebx), %ebp
+
+	movl    BP_init_size(%esi), %eax
+	subl    $_end, %eax
+	movl    %ebx, %ebp
+	subl    %eax, %ebp
 	pushl	%ebp		/* output address */
+
 	pushl	$z_input_len	/* input_len */
 	leal	input_data(%ebx), %eax
 	pushl	%eax		/* input_data */
Index: linux-2.6/arch/x86/boot/compressed/mkpiggy.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/mkpiggy.c
+++ linux-2.6/arch/x86/boot/compressed/mkpiggy.c
@@ -83,11 +83,8 @@ int main(int argc, char *argv[])
 	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_extract_offset\n");
-	printf("z_extract_offset = 0x%lx\n", offs);
-	/* z_extract_offset_negative allows simplification of head_32.S */
-	printf(".globl z_extract_offset_negative\n");
-	printf("z_extract_offset_negative = -0x%lx\n", offs);
+	printf(".globl z_min_extract_offset\n");
+	printf("z_min_extract_offset = 0x%lx\n", offs);
 	printf(".globl z_run_size\n");
 	printf("z_run_size = %lu\n", run_size);
 
Index: linux-2.6/arch/x86/boot/header.S
===================================================================
--- linux-2.6.orig/arch/x86/boot/header.S
+++ linux-2.6/arch/x86/boot/header.S
@@ -440,12 +440,22 @@ setup_data:		.quad 0			# 64-bit physical
 
 pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 
-#define ZO_INIT_SIZE	(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
+# don't overlap data area of ZO with VO
+#define ADDON_ZO_SIZE (ZO__end - ZO__rodata)
+
+#define ZO_INIT_SIZE	(ZO__end - ZO_startup_32 + ZO_z_min_extract_offset)
 #define VO_INIT_SIZE	(VO__end - VO__text)
 #if ZO_INIT_SIZE > VO_INIT_SIZE
+
+/* only add the difference to cover ADDON_ZO */
+#if (ZO_INIT_SIZE - VO_INIT_SIZE) < ADDON_ZO_SIZE
+#define INIT_SIZE (ZO_INIT_SIZE + (ADDON_ZO_SIZE-(ZO_INIT_SIZE - VO_INIT_SIZE)))
+#else
 #define INIT_SIZE ZO_INIT_SIZE
+#endif
+
 #else
-#define INIT_SIZE VO_INIT_SIZE
+#define INIT_SIZE (VO_INIT_SIZE + ADDON_ZO_SIZE)
 #endif
 init_size:		.long INIT_SIZE		# kernel initialization size
 handover_offset:	.long 0			# Filled in by build.c
Index: linux-2.6/arch/x86/boot/compressed/vmlinux.lds.S
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/vmlinux.lds.S
+++ linux-2.6/arch/x86/boot/compressed/vmlinux.lds.S
@@ -35,6 +35,7 @@ SECTIONS
 		*(.text.*)
 		_etext = . ;
 	}
+        . = ALIGN(PAGE_SIZE); /* keep ADDON_ZO_SIZE page aligned */
 	.rodata : {
 		_rodata = . ;
 		*(.rodata)	 /* read-only data */
@@ -70,5 +71,6 @@ SECTIONS
 		_epgtable = . ;
 	}
 #endif
+	. = ALIGN(PAGE_SIZE);	/* keep ZO size page aligned */
 	_end = .;
 }
Index: linux-2.6/arch/x86/kernel/vmlinux.lds.S
===================================================================
--- linux-2.6.orig/arch/x86/kernel/vmlinux.lds.S
+++ linux-2.6/arch/x86/kernel/vmlinux.lds.S
@@ -325,6 +325,7 @@ SECTIONS
 		__brk_limit = .;
 	}
 
+	. = ALIGN(PAGE_SIZE);		/* keep VO_INIT_SIZE page aligned */
 	_end = .;
 
         STABS_DEBUG
Index: linux-2.6/arch/x86/boot/Makefile
===================================================================
--- linux-2.6.orig/arch/x86/boot/Makefile
+++ linux-2.6/arch/x86/boot/Makefile
@@ -86,7 +86,7 @@ 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\|z_.*\)$$/\#define ZO_\2 0x\1/p'
+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\|_rodata\|z_.*\)$$/\#define ZO_\2 0x\1/p'
 
 quiet_cmd_zoffset = ZOFFSET $@
       cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] x86, boot: Make data from decompress_kernel stage live longer
  2015-03-15  7:49 [PATCH v4] x86, boot: Make data from decompress_kernel stage live longer Yinghai Lu
@ 2015-03-16  7:45 ` Ingo Molnar
  2015-03-16 19:25   ` Yinghai Lu
  0 siblings, 1 reply; 3+ messages in thread
From: Ingo Molnar @ 2015-03-16  7:45 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: H. Peter Anvin, Ingo Molnar, Kees Cook, Borislav Petkov,
	Baoquan He, Thomas Gleixner, Jiri Kosina, Andrew Morton,
	Linus Torvalds, linux-kernel, Matt Fleming


* Yinghai Lu <yinghai@kernel.org> wrote:

> After commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address 
> calculation"), get warning about setup_data from debugfs for 
> setup_data. Boris found setup_data for kaslr from boot stage become 
> all 0's in kernel stage.

And? What is the high level effect to the user?

'setup_data for kaslr from boot stage becomes all 0's' is not a high 
level effect. It's a detail of the mechanism of the bug, not a high 
level effect.

Think of a regular Linux user reading your changelogs. The first 
paragraph of bug fixes should be readable to them!

_That_ is what 'high level' means.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] x86, boot: Make data from decompress_kernel stage live longer
  2015-03-16  7:45 ` Ingo Molnar
@ 2015-03-16 19:25   ` Yinghai Lu
  0 siblings, 0 replies; 3+ messages in thread
From: Yinghai Lu @ 2015-03-16 19:25 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: H. Peter Anvin, Ingo Molnar, Kees Cook, Borislav Petkov,
	Baoquan He, Thomas Gleixner, Jiri Kosina, Andrew Morton,
	Linus Torvalds, Linux Kernel Mailing List, Matt Fleming

On Mon, Mar 16, 2015 at 12:45 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Yinghai Lu <yinghai@kernel.org> wrote:
>
>> After commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address
>> calculation"), get warning about setup_data from debugfs for
>> setup_data. Boris found setup_data for kaslr from boot stage become
>> all 0's in kernel stage.
>
> And? What is the high level effect to the user?
>
> 'setup_data for kaslr from boot stage becomes all 0's' is not a high
> level effect. It's a detail of the mechanism of the bug, not a high
> level effect.
>
> Think of a regular Linux user reading your changelogs. The first
> paragraph of bug fixes should be readable to them!

That should be ioremap warning that reported by
http://marc.info/?l=linux-kernel&m=142492905425130&w=2

and later Boris found those all 0's.

Thanks

Yinghai

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-03-16 19:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-15  7:49 [PATCH v4] x86, boot: Make data from decompress_kernel stage live longer Yinghai Lu
2015-03-16  7:45 ` Ingo Molnar
2015-03-16 19:25   ` Yinghai Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).