Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly

From: Yinghai Lu
Date: Mon Mar 02 2015 - 17:10:30 EST

Next message: Nick Meier: "RE: [char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000000000000000 is so big it is unsigned long"
Previous message: Rafael J. Wysocki: "Re: [PATCH 1/2] cpuidle: Clean up fallback handling in cpuidle_idle_call()"
In reply to: Borislav Petkov: "Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly"
Next in thread: Borislav Petkov: "Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Mon, Mar 2, 2015 at 12:25 PM, Borislav Petkov <bp@xxxxxxx> wrote:

> unsigned char *choose_kernel_location(struct boot_params *params,
> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
> index 34d047c98284..26d62f4b27b9 100644
> --- a/arch/x86/boot/compressed/vmlinux.lds.S
> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
> @@ -29,6 +29,10 @@ SECTIONS
> .rodata..compressed : {
> *(.rodata..compressed)
> }
> + .setup_data : {
> + _setup_data = . ;
> + *(.setup_data)
> + }
> .text : {
> _text = .; /* Text */
> *(.text)

That does not help, we will still have overlap between copied Zo
vmlinux and decompressed Vo vmlinux

because the Zo vmlinux is copied to middle of buffer instead of end of
the buffer.

definition:
Zo vmliunx: arch/x86/boot/compressed/vmlinux, from
arch/x86/boot/compressed/vmlinux.lds.S
Vo vmlinux: vmlinux from arch/x86/kernel/vmlinux.lds.S

BTW, found more problem about run_size for kaslr, we should use
init_size instead.

Here the three patches that should go into v4.0. Help Ingo will be
happy with updated change log.

Thanks

Yinghai
Subject: [PATCH] x86, boot, aslr: Use init_size instead of run_size

commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")

introduced one run_size for kaslr.

We do not need to have home grown run_size.

We should use real runtime size (include copy/decompress) aka init_size

Please check arch/x86/boot/header.S about init_size for detail.

Fixes: e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd")
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Josh Triplett <josh@xxxxxxxxxxxxxxxx>
Cc: Matt Fleming <matt.fleming@xxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Cc: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
Cc: Junjie Mao <eternal.n08@xxxxxxxxx>
Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>

---
arch/x86/boot/compressed/Makefile | 4 ---
arch/x86/boot/compressed/head_32.S | 5 +---
arch/x86/boot/compressed/head_64.S | 5 ----
arch/x86/boot/compressed/misc.c | 15 ++++++-------
arch/x86/boot/compressed/mkpiggy.c | 9 +------
arch/x86/tools/calc_run_size.sh | 42 -------------------------------------
6 files changed, 13 insertions(+), 67 deletions(-)

Index: linux-2.6/arch/x86/boot/compressed/Makefile
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/Makefile
+++ linux-2.6/arch/x86/boot/compressed/Makefile
@@ -92,10 +92,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
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
@@ -208,8 +208,7 @@ 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 */
+ pushl $z_output_len /* decompressed length */
leal z_extract_offset_negative(%ebx), %ebp
pushl %ebp /* output address */
pushl $z_input_len /* input_len */
@@ -219,7 +218,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.
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
@@ -403,16 +403,13 @@ 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 */
movl $z_input_len, %ecx /* input_len */
movq %rbp, %r8 /* output target address */
- movq $z_output_len, %r9 /* decompressed length, end of relocs */
+ movq $z_output_len, %r9 /* decompressed length */
call decompress_kernel /* returns kernel location in %rax */
- popq %r9
popq %rsi

/*
Index: linux-2.6/arch/x86/boot/compressed/misc.c
===================================================================
--- linux-2.6.orig/arch/x86/boot/compressed/misc.c
+++ linux-2.6/arch/x86/boot/compressed/misc.c
@@ -370,10 +370,10 @@ asmlinkage __visible void *decompress_ke
unsigned char *input_data,
unsigned long input_len,
unsigned char *output,
- unsigned long output_len,
- unsigned long run_size)
+ unsigned long output_len)
{
unsigned char *output_orig = output;
+ unsigned long init_size;

real_mode = rmode;

@@ -396,15 +396,14 @@ asmlinkage __visible void *decompress_ke
free_mem_ptr = heap; /* Heap */
free_mem_end_ptr = heap + BOOT_HEAP_SIZE;

+ init_size = real_mode->hdr.init_size;
+
/*
- * The memory hole needed for the kernel is the larger of either
- * the entire decompressed kernel plus relocation table, or the
- * entire decompressed kernel plus .bss and .brk sections.
+ * The memory hole needed for the kernel is init_size for running
+ * and init_size is bigger than output_len always.
*/
output = choose_kernel_location(real_mode, input_data, input_len,
- output,
- output_len > run_size ? output_len
- : run_size);
+ output, init_size);

/* Validate memory location choices. */
if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
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
@@ -36,13 +36,11 @@ int main(int argc, char *argv[])
uint32_t olen;
long ilen;
unsigned long offs;
- 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;
}

@@ -76,7 +74,6 @@ int main(int argc, char *argv[])
offs += olen >> 12; /* Add 8 bytes for each 32K block */
offs += 64*1024 + 128; /* Add 64K + 128 bytes slack */
offs = (offs+4095) & ~4095; /* Round to a 4K boundary */
- run_size = atoi(argv[2]);

printf(".section \".rodata..compressed\",\"a\",@progbits\n");
printf(".globl z_input_len\n");
@@ -88,8 +85,6 @@ int main(int argc, char *argv[])
/* 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_run_size\n");
- printf("z_run_size = %lu\n", run_size);

printf(".globl input_data, input_data_end\n");
printf("input_data:\n");
Index: linux-2.6/arch/x86/tools/calc_run_size.sh
===================================================================
--- linux-2.6.orig/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
Subject: [PATCH v2] x86, boot: keep data from boot stage to kernel stage.

bp found data from boot stage can not be used kernel stage.

Acctually those data area is overlapped with kernel bss stage, and clear_bss()
clear them before code in arch/x86/kernel/setup.c access them.

To make the data survive that later, we should avoid the overlapping.

Need to:
1. move compressed kernel close the end of buffer instead of middle of it.
2. extend init_size so no one from kernel bss and brk will touch the data
region from boot/compressed/misc.c

-v2: add init_size in arch/x86/boot/header.S instead of BRK.

Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: Matt Fleming <matt.fleming@xxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>

---
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 | 3 ---
arch/x86/boot/compressed/vmlinux.lds.S | 2 ++
arch/x86/boot/header.S | 7 +++++--
arch/x86/kernel/asm-offsets.c | 1 +
7 files changed, 24 insertions(+), 10 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
@@ -102,7 +102,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
@@ -330,7 +332,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
@@ -148,7 +148,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_output_len /* decompressed length */
- 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
@@ -82,9 +82,6 @@ int main(int argc, char *argv[])
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 input_data, input_data_end\n");
printf("input_data:\n");
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) > $@
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 ADDON_ZO_SIZE page aligned */
_end = .;
}
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,15 @@ setup_data: .quad 0 # 64-bit physical

pref_address: .quad LOAD_PHYSICAL_ADDR # preferred load addr

+# 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_extract_offset)
#define VO_INIT_SIZE (VO__end - VO__text)
#if ZO_INIT_SIZE > VO_INIT_SIZE
-#define INIT_SIZE ZO_INIT_SIZE
+#define INIT_SIZE (ZO_INIT_SIZE + ADDON_ZO_SIZE)
#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
Subject: [PATCH v2] x86, kaslr: get kaslr_enabled back correctly

commit f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
is using address as vaule for kaslr_enabled.

That will random kaslr_enabled get that set or cleared.
Will have problem for system really have kaslr enabled.

-v2: update changelog.

Fixes: f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")
Cc: Matt Fleming <matt.fleming@xxxxxxxxx>
Cc: Borislav Petkov <bp@xxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Jiri Kosina <jkosina@xxxxxxx>
Acked-by: Jiri Kosina <jkosina@xxxxxxx>
Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>

---
arch/x86/kernel/setup.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

Index: linux-2.6/arch/x86/kernel/setup.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup.c
+++ linux-2.6/arch/x86/kernel/setup.c
@@ -429,7 +429,13 @@ static void __init reserve_initrd(void)

static void __init parse_kaslr_setup(u64 pa_data, u32 data_len)
{
- kaslr_enabled = (bool)(pa_data + sizeof(struct setup_data));
+ /* kaslr_setup_data is defined in aslr.c */
+ unsigned char *data;
+ unsigned long offset = sizeof(struct setup_data);
+
+ data = early_memremap(pa_data, offset + 1);
+ kaslr_enabled = *(data + offset);
+ early_memunmap(data, offset + 1);
}

static void __init parse_setup_data(void)

Next message: Nick Meier: "RE: [char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000000000000000 is so big it is unsigned long"
Previous message: Rafael J. Wysocki: "Re: [PATCH 1/2] cpuidle: Clean up fallback handling in cpuidle_idle_call()"
In reply to: Borislav Petkov: "Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly"
Next in thread: Borislav Petkov: "Re: [PATCH 1/8] x86, kaslr: get kaslr_enabled back correctly"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]