x86, boot: clean up kasl

From arch/x86/boot/header.S:

On Sat, Mar 07, 2015 at 02:07:15PM -0800, Yinghai Lu wrote:

...

I ended up committing this. Anything I've missed?

So this would be fine as-is for v4.1, but I think we want to attempt to fix this in x86/urgent so that we don't have to revert the kaslr changes in v4.0, so it would be awesome if you could split it into two
parts: the fix, plus the orphaned run_size removal?

I ended up committing this. Anything I've missed?

---
From: Yinghai Lu <[email protected]>
Date: Sat, 7 Mar 2015 14:07:15 -0800
Subject: [PATCH] x86/setup: Use init_size instead of run_size

Commit

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

introduced run_size for KASLR to represent the size of kernel proper (vmlinux).

However, we should use the actual runtime size (which provides for copy/decompress), i.e. init_size, as it includes .bss and .brk.

Why, you ask?

Because init_size is the size needed for safe kernel decompression and thus can be higher than run_size in case the decompressor needs a larger buffer.

From arch/x86/boot/header.S:
#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
init_size: .long INIT_SIZE # kernel initialization size

The boot loader allocates a buffer of size init_size which it reads from the setup header and loads the compressed kernel (arch/x86/boot/compressed/vmlinux) in it.

init_size initially comes from the kernel proper's (vmlinux) init size. It includes the .bss and .brk area.

When the boot loader hands off to the compressed kernel, the last moves itself to z_extract_offset within the buffer to make sure that the decompressor output does not overwrite input data before it gets consumed.

However, z_extract_offset is the size difference
between the uncompressed and compressed kernel (see arch/x86/boot/compressed/mkpiggy.c) and thus represents the additional space needed for decompression but it doesn't factor in a bigger ZO_INIT_SIZE.

During ASLR buffer searching, we need to make sure the new buffer is big enough for decompression. So use init_size instead, and kill run_size related code.

Can you put back:
"
So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.

Quoted 5 lines · Yinghai Lu

Can you put back:
"
So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.

Why?

We don't adjust init_size. We assign INIT_SIZE to it. Or do you want to say that we indirectly adjust init_size through INIT_SIZE?

Yes. Just to emphasize that " We need to make sure [z_extra_offset, init_size) will fit ZO"

Quoted 2 lines · Yinghai Lu

Yes. Just to emphasize that " We need to make sure [z_extra_offset, init_size) will fit ZO"

So you want to say:

"We need to make sure the compressed kernel fits in the interval [z_extra_offset, z_extra_offset + init_size)"

commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd") introduced one run_size for kaslr.
We should use real runtime size (include copy/decompress) aka init_size.

run_size is VO (vmlinux) init size include bss and brk. init_size is the size needed for decompress and it is bigger than run_size when decompress need more buff.

According to arch/x86/boot/header.S:

| #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
| init_size:              .long INIT_SIZE         # kernel initialization size

Bootloader allocate buffer according to init_size in hdr, and load the ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer. init_size first come from VO (vmlinux) init size. That VO init size is from VO _end to VO _end and include VO bss and brk area.

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.
But z_extract_offset calculating is based on size of VO (vmlinux) and size of compressed VO only at first.
So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.

During aslr buffer searching, we need to make sure the new buffer is big enough for decompress at first. So use init_size instead, and kill not needed run_size related code.

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

Cc "H. Peter Anvin" <[email protected]>

Cc Josh Triplett <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Kees Cook <[email protected]>

Cc Andrew Morton <akpm-de/[email protected]>

Cc Ard Biesheuvel <[email protected]>

Cc Junjie Mao <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

---
 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(-)
 delete mode 100644 arch/x86/tools/calc_run_size.sh

arch/x86/boot/compressed/Makefile +1 -3

index 0a291cd..70cc92c 100644
@@ -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

arch/x86/boot/compressed/head_32.S +2 -3

index 1d7fbbc..cbed140 100644
@@ -207,8 +207,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 */

@@ -218,7 +217,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.

arch/x86/boot/compressed/head_64.S +1 -4

index 6b1766c..2884e0c 100644
@@ -402,16 +402,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
 
 /*

arch/x86/boot/compressed/misc.c +7 -8

index 5903089..51e9e54 100644
@@ -370,10 +370,10 @@ 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)
 {
        unsigned char *output_orig = output;
+       unsigned long init_size;
 
        real_mode = rmode;
 

@@ -396,15 +396,14 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
        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))

arch/x86/boot/compressed/mkpiggy.c +2 -7

index d8222f2..b669ab6 100644
@@ -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");

arch/x86/tools/calc_run_size.sh deleted +0 -43

deleted file mode 100644
index 1a4c17b..0000000
@@ -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
--

1.8.4.5

commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd") introduced one run_size for kaslr.
We should use real runtime size (include copy/decompress) aka init_size.

run_size is VO (vmlinux) init size include bss and brk. init_size is the size needed for decompress and it is bigger than run_size when decompress need more buff.

According to arch/x86/boot/header.S:

| #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
| init_size:              .long INIT_SIZE         # kernel initialization size

Bootloader allocate buffer according to init_size in hdr, and load the ZO (arch/x86/boot/compressed/vmlinux) from start of that buffer. init_size first come from VO (vmlinux) init size. That VO init size is from VO _end to VO _end and include VO bss and brk area.

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.
But z_extract_offset calculating is based on size of VO (vmlinux) and size of compressed VO only at first.
So need to make sure [z_extra_offset, init_size) will fit ZO, that means init_size need to be adjusted according to ZO size.
That make init_size is always >= run_size.

During aslr buffer searching, we need to make sure the new buffer is big enough for decompress at first. So use init_size instead, and kill not needed run_size related code.

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

Quoted 3 lines · Yinghai Lu

commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd") introduced one run_size for kaslr.
We should use real runtime size (include copy/decompress) aka init_size.

Why, what happens if we don't change this?

Quoted 7 lines · Ingo Molnar

Quoted 3 lines · Yinghai Lu

commit e6023367d779 ("x86, kaslr: Prevent .bss from overlaping initrd") introduced one run_size for kaslr. We should use real runtime size (include copy/decompress) aka init_size.

Quoted 1 line

Why, what happens if we don't change this?

While trying to update change log, found we still need to keep this run_size. otherwise kaslr will find not needed big size of init_size instead of max(output_len, run_size).

will refresh the patchset.

From 53da3bc2ba9e4899f32707b5cd7d18421b943687 Mon Sep 17 00:00:00 2001

You are still talking only about the low level code and about low level symptoms, while in contrast to that the primary question with...

In addition to what you've said, I have only one proposal: we revert the whole crap and do it again, this time nice and clean in tip. It'll stay there as long as it has to. No half-arsed commit messages, no misunderstood crap, no lala code f*ckery. Whatever it takes, as long as it takes.

Commit-ID:  69797dafe35541bfff1989c0b37c66ed785faf0e
Gitweb:     http://git.kernel.org/tip/69797dafe35541bfff1989c0b37c66ed785faf0e
Author:     Borislav Petkov <[email protected]>
AuthorDate: Mon, 16 Mar 2015 11:06:28 +0100
Committer:  Ingo Molnar <[email protected]>
CommitDate: Mon, 16 Mar 2015 11:18:21 +0100

Revert "x86/mm/ASLR: Propagate base load address calculation"

This reverts commit:

f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

The main reason for the revert is that the new boot flag does not work at all currently, and in order to make this work, we need non-trivial changes to the x86 boot code which we didn't manage to get done in time for merging.

And even if we did, they would've been too risky so instead of rushing things and break booting 4.1 on boxes left and right, we will be very strict and conservative and will take our time with this to fix and test it properly.

Reported-by Yinghai Lu <[email protected]>

Link http://lkml.kernel.org/r/[email protected] open here

Quoted 5 lines · Borislav Petkov

In addition to what you've said, I have only one proposal: we revert the whole crap and do it again, this time nice and clean in tip. It'll stay there as long as it has to. No half-arsed commit messages, no misunderstood crap, no lala code f*ckery. Whatever it takes, as long as it takes.

And here it is:

---
From: Borislav Petkov <[email protected]>
Date: Mon, 16 Mar 2015 10:57:56 +0100
Subject: [PATCH] Revert "x86/mm/ASLR: Propagate base load address calculation"

This reverts commit f47233c2d34f243ecdaac179c3408a39ff9216a7.

The main reason for the revert is that in order to make this work, we need non-trivial changes to the x86 boot code which we didn't manage to get done in time for merging.

And even if we did, they would've been too risky so instead of rushing things and break booting 4.1 on boxes left and right, we will be very strict and conservative and will take our time with this to fix and test it properly.

Signed-off-by Borislav Petkov <[email protected]>

Cc Jiri Kosina <[email protected]>

Quoted 5 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>
Date: Mon, 16 Mar 2015 10:57:56 +0100
Subject: [PATCH] Revert "x86/mm/ASLR: Propagate base load address calculation"

This reverts commit f47233c2d34f243ecdaac179c3408a39ff9216a7.

Agreed. Let's work on a better refresh for 4.1+. The fix attempts were quite chaotic.

Quoted 5 lines · Jiri Kosina

Quoted 4 lines · Borislav Petkov

From: Borislav Petkov <[email protected]>
Date: Mon, 16 Mar 2015 10:57:56 +0100
Subject: [PATCH] Revert "x86/mm/ASLR: Propagate base load address calculation"

Quoted 1 line

Quoted 1 line

This reverts commit f47233c2d34f243ecdaac179c3408a39ff9216a7.

...

Quoted 3 lines

Agreed. Let's work on a better refresh for 4.1+. The fix attempts were quite chaotic.

Then what is the plan for 4.1+ ?

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

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 called, VO (vmlinux) use whole buffer from start, and ZO code and data section is overlapped with VO bss section. And later VO/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. At first move ZO close the end of buffer instead of middle of the buffer, that will move out ZO data out of VO bss area.

Also after that we can find out where is data section of copied ZO instead of guessing. That will aslr mem_avoid array filling for

new buffer seaching much simple.

And rename z_extract_offset to z_min_extract_offset, as it is actually the minimum offset for extracting now.

To keep the final real extract_offset to be page aligned like min_extract_offset, we need make VO _end and ZO _end both page aligned to make sure init_size always page aligned.

Next patch will add ZO data size to init_size, so it will make sure ZO data is even out of VO brk area.

Fixes f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

Cc "H. Peter Anvin" <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Kees Cook <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

---
 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 |  1 +
 arch/x86/boot/header.S                 |  2 +-
 arch/x86/kernel/asm-offsets.c          |  1 +
 arch/x86/kernel/vmlinux.lds.S          |  1 +
 7 files changed, 21 insertions(+), 10 deletions(-)

arch/x86/boot/compressed/head_32.S +9 -2

index cbed140..a9b56f1 100644
@@ -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

@@ -208,8 +210,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 */

arch/x86/boot/compressed/head_64.S +6 -2

index 2884e0c..69015b5 100644
@@ -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

arch/x86/boot/compressed/mkpiggy.c +2 -5

index b669ab6..c03b009 100644
@@ -80,11 +80,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 input_data, input_data_end\n");
        printf("input_data:\n");

arch/x86/boot/compressed/vmlinux.lds.S +1 -0

index 34d047c..e24e0a0 100644
@@ -70,5 +70,6 @@ SECTIONS
                _epgtable = . ;
        }
 #endif
+       . = ALIGN(PAGE_SIZE);   /* keep ZO size page aligned */
        _end = .;
 }

arch/x86/boot/header.S +1 -1

index 16ef025..9bfab22 100644
@@ -440,7 +440,7 @@ setup_data:         .quad 0                 # 64-bit physical pointer to
 
 pref_address:          .quad LOAD_PHYSICAL_ADDR        # preferred load addr
 
-#define ZO_INIT_SIZE   (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
+#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
 #define INIT_SIZE ZO_INIT_SIZE

arch/x86/kernel/asm-offsets.c +1 -0

index 9f6b934..0e8e4f7 100644
@@ -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);
 

arch/x86/kernel/vmlinux.lds.S +1 -1

index 00bf300..ac25c7f 100644
@@ -325,6 +325,7 @@ SECTIONS
                __brk_limit = .;
        }
 
+       . = ALIGN(PAGE_SIZE);           /* keep VO init size page aligned */
        _end = .;
 
         STABS_DEBUG
--

1.8.4.5

Quoted 1 line · Yinghai Lu

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

"... be used during kernel stage."

Also, can you give a specific example of this problem? (Which data, used how?)

Quoted 10 lines

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 called, VO (vmlinux) use whole buffer from start, and ZO code and data section is overlapped with VO bss section. And later VO/clear_bss() clear them before code in arch/x86/kernel/setup.c access them.

Quoted 6 lines

To make the data survive that later, we should avoid the overlapping. At first move ZO close the end of buffer instead of middle of the buffer, that will move out ZO data out of VO bss area.

Also after that we can find out where is data section of copied ZO instead of guessing. That will aslr mem_avoid array filling for

"That will make aslr mem_avoid array ..."

Quoted 9 lines

new buffer seaching much simple.

And rename z_extract_offset to z_min_extract_offset, as it is actually the minimum offset for extracting now.

To keep the final real extract_offset to be page aligned like min_extract_offset, we need make VO _end and ZO _end both page aligned to make sure init_size always page aligned.

Quoted 2 lines

Next patch will add ZO data size to init_size, so it will make sure ZO data is even out of VO brk area.

This seems like a reasonable idea, but I think the changes should be noted/updated in misc.c since a lot of effort was made to make the in-memory foot print as small as possible.

These changes do expand the
size of the loaded kernel, IIUC. If not in this patch, maybe in 5/7?

Quoted 1 line · Yinghai Lu

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

"... be used during kernel stage."

Also, can you give a specific example of this problem? (Which data, used how?)

Final patch:

---
From: Yinghai Lu <[email protected]>
Date: Sat, 7 Mar 2015 14:07:16 -0800
Subject: [PATCH] x86/setup: Move compressed kernel to the end of the buffer

Signed-off-by Yinghai Lu <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Kees Cook <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jiri Kosina <[email protected]>

Cc [email protected]

Cc Ingo Molnar <[email protected]>

Cc Baoquan He <[email protected]>

Fixes f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

Link http://lkml.kernel.org/r/1425766041-6551-3-git-send-email-yinghai-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org open here

[ Commit message massively rewritten ]

Signed-off-by:

---
 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 |  1 +
 arch/x86/boot/header.S                 |  2 +-
 arch/x86/kernel/asm-offsets.c          |  1 +
 arch/x86/kernel/vmlinux.lds.S          |  1 +
 7 files changed, 21 insertions(+), 10 deletions(-)

arch/x86/boot/compressed/head_32.S +9 -2

index cbed1407a5cd..a9b56f1d8e75 100644
@@ -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

@@ -208,8 +210,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 */

arch/x86/boot/compressed/head_64.S +6 -2

index 2884e0c3e8a5..69015b576cf6 100644
@@ -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

arch/x86/boot/compressed/mkpiggy.c +2 -5

index b669ab65bf6c..c03b0097ce58 100644
@@ -80,11 +80,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 input_data, input_data_end\n");
 	printf("input_data:\n");

arch/x86/boot/compressed/vmlinux.lds.S +1 -0

index 34d047c98284..a80acabb80ec 100644
@@ -70,5 +70,6 @@ SECTIONS
 		_epgtable = . ;
 	}
 #endif
+	. = ALIGN(PAGE_SIZE);	/* keep size page-aligned */
 	_end = .;
 }

arch/x86/boot/header.S +1 -1

index 16ef02596db2..9bfab22efdf7 100644
@@ -440,7 +440,7 @@ setup_data:		.quad 0			# 64-bit physical pointer to
 
 pref_address:		.quad LOAD_PHYSICAL_ADDR	# preferred load addr
 
-#define ZO_INIT_SIZE	(ZO__end - ZO_startup_32 + ZO_z_extract_offset)
+#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
 #define INIT_SIZE ZO_INIT_SIZE

arch/x86/kernel/asm-offsets.c +1 -0

index 9f6b9341950f..0e8e4f7a31ce 100644
@@ -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);
 

arch/x86/kernel/vmlinux.lds.S +1 -0

index 00bf300fd846..a92d3dc2812a 100644
@@ -325,6 +325,7 @@ SECTIONS
 		__brk_limit = .;
 	}
 
+	. = ALIGN(PAGE_SIZE);		/* keep init size page-aligned */
 	_end = .;
 
         STABS_DEBUG

Signature · 9 lines

2.2.0.33.gc18b867


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

Thanks a lot for fixing my oversight.

Final patch:

---
From: Yinghai Lu <[email protected]>
Date: Sat, 7 Mar 2015 14:07:16 -0800
Subject: [PATCH] x86/setup: Move compressed kernel to the end of the buffer

Also stop using "compressed kernel" please, that is confusing.

Just use

ZO: arch/x86/boot/compressed/vmlinux
VO: vmlinux

Final patch:

---
From: Yinghai Lu <[email protected]>
Date: Sat, 7 Mar 2015 14:07:16 -0800
Subject: [PATCH] x86/setup: Move compressed kernel to the end of the buffer

Boris found that passing KASLR status through setup_data from the boot stage cannot be used later in the kernel stage, see commit

f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

Here's some background:

The boot loader allocates a buffer of size init_size in concordance with the value passed in the setup header and it loads the compressed, i.e. first kernel (arch/x86/boot/compressed/vmlinux) in it.

First kernel then moves itself somewhere around the middle of the buffer at z_extract_offset to make sure that the decompressor does not overwrite input data.

After the decompressor is finished, kernel proper (vmlinux) uses the whole buffer from the beginning and the compressed kernel's code and data section is overlapped with the kernel proper's bss section.

Later on, clear_bss() in kernel proper clears .bss before code in arch/x86/kernel/setup.c can access setup_data passed in the first, compressed kernel.

To make sure that data survives, we should avoid the overlapping.

As a first step, move the first kernel closer to the end of the buffer instead of the middle. As a result, this will place first kernel's data area out of kernel proper's .bss area.

This way we can find out where the data section of the copied first kernel is instead of guessing. In addition, it will make the KASLR mem_avoid array preparation for the search of a fitting buffer much simpler.

While at it, rename z_extract_offset to z_min_extract_offset as it is actually the minimum extract offset now.

In order to keep the final extract offset page-aligned we need to make both kernels' _end markers page-aligned too so that init_size is page-aligned as a result.

Signed-off-by Yinghai Lu <[email protected]>

Cc "H. Peter Anvin" <[email protected]>

Cc Matt Fleming <[email protected]>

Cc Kees Cook <[email protected]>

Cc Thomas Gleixner <[email protected]>

Cc Jiri Kosina <[email protected]>

Cc [email protected]

Cc Ingo Molnar <[email protected]>

Cc Baoquan He <[email protected]>

Fixes f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

Link http://lkml.kernel.org/r/[email protected] open here

[ Commit message massively rewritten ]

Signed-off-by:

---
 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 |  1 +
 arch/x86/boot/header.S                 |  2 +-
 arch/x86/kernel/asm-offsets.c          |  1 +
 arch/x86/kernel/vmlinux.lds.S          |  1 +
 7 files changed, 21 insertions(+), 10 deletions(-)

arch/x86/boot/compressed/head_32.S +9 -2

index cbed1407a5cd..a9b56f1d8e75 100644
@@ -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

@@ -208,8 +210,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 */

arch/x86/boot/compressed/head_64.S +6 -2

index 2884e0c3e8a5..69015b576cf6 100644
@@ -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

arch/x86/boot/compressed/mkpiggy.c +2 -5

index b669ab65bf6c..c03b0097ce58 100644
@@ -80,11 +80,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 input_data, input_data_end\n");
        printf("input_data:\n");

arch/x86/boot/compressed/vmlinux.lds.S +1 -0

index 34d047c98284..a80acabb80ec 100644
@@ -70,5 +70,6 @@ SECTIONS
                _epgtable = . ;
        }
 #endif
+       . = ALIGN(PAGE_SIZE);   /* keep size page-aligned */
        _end = .;
 }

arch/x86/boot/header.S +1 -1

index 16ef02596db2..9bfab22efdf7 100644
@@ -440,7 +440,7 @@ setup_data:         .quad 0                 # 64-bit physical pointer to
 
 pref_address:          .quad LOAD_PHYSICAL_ADDR        # preferred load addr
 
-#define ZO_INIT_SIZE   (ZO__end - ZO_startup_32 + ZO_z_extract_offset)
+#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
 #define INIT_SIZE ZO_INIT_SIZE

arch/x86/kernel/asm-offsets.c +1 -0

index 9f6b9341950f..0e8e4f7a31ce 100644
@@ -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);
 

arch/x86/kernel/vmlinux.lds.S +1 -1

index 00bf300fd846..a92d3dc2812a 100644
@@ -325,6 +325,7 @@ SECTIONS
                __brk_limit = .;
        }
 
+       . = ALIGN(PAGE_SIZE);           /* keep init size page-aligned */
        _end = .;
 
         STABS_DEBUG
--

2.2.0.33.gc18b867

--
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--

Final patch:

---
From: Yinghai Lu <[email protected]>
Date: Sat, 7 Mar 2015 14:07:17 -0800
Subject: [PATCH] x86/setup: Don't overlap vmlinux's brk with compressed kernel's data

We need to keep VO and ZO here...

Quoted 1 line · Yinghai Lu

We need to keep VO and ZO here...

Why?

Make it not confusing.

ZO: arch/x86/boot/compressed/vmlinux
VO: vmlinux

setup + ZO ==> bzImage.

compressed kernel is just compressed VO.

Quoted 8 lines · Yinghai Lu

Make it not confusing.

ZO: arch/x86/boot/compressed/vmlinux
VO: vmlinux

setup + ZO ==> bzImage.

compressed kernel is just compressed VO.

So the two end up being the "compressed kernel" and "kernel proper".

And then the subject turns into

"Don't overlap kernel proper's brk with the compressed kernel's data section"

Does that still confuse you?

In arch/x86/boot/header.S, we already use VO and ZO.
So please keep on using them, and don't introduce "kernel proper" etc.

Quoted 2 lines · Yinghai Lu

In arch/x86/boot/header.S, we already use VO and ZO.
So please keep on using them, and don't introduce "kernel proper" etc.

So you're suggesting commit messages should use variable names and prefixes from the code instead of being human-readable?

Commit

f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

started passing KASLR status to kernel proper, but it uses a physical address as the vaule, leading to parsing bogus KASLR status in kernel proper.

The setup_data linked list and thus the element which contains kaslr_enabled is chained together using physical addresses. At the time when we access it in the kernel proper, we're already running with paging enabled and therefore must access it through its virtual address.

This patch changes the code to use early_memmap() and access the value.

Fixes f47233c2d34f ("x86/mm/ASLR: Propagate base load address calculation")

Cc Matt Fleming <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Kees Cook <[email protected]>

Cc Jiri Kosina <[email protected]>

Acked-by Jiri Kosina <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

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

arch/x86/kernel/setup.c +12 -2

index 98dc931..912f124 100644
@@ -429,7 +429,18 @@ 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);
+       if (!data) {
+               kaslr_enabled = true;
+               return;
+       }
+
+       kaslr_enabled = *(data + offset);
+       early_memunmap(data, offset + 1);
 }
 
 static void __init parse_setup_data(void)
--

1.8.4.5

Now ZO sit end of the buffer, we can find out where is ZO text and data/bss etc.

[input, input+input_size) is copied compressed kernel, not the whole ZO. [output, output+init_size) is the buffer for VO.

[input+input_size, output+init_size) is [_text, _end) for ZO. that could be first range in mem_avoid. We don't need to guess that anymore.

That area already include heap and stack for ZO running. So we don't need to put them into mem_avoid array.

We need to put boot_params into the mem_avoid too. As with 64bit bootloader could put it anywhere.

Also change output_size referring to init_size, as we pass init_size instead.

Cc Kees Cook <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/boot/compressed/aslr.c | 29 ++++++++++++++---------------
 arch/x86/boot/compressed/misc.h |  4 ++--
 2 files changed, 16 insertions(+), 17 deletions(-)

arch/x86/boot/compressed/aslr.c +14 -15

index 7083c16..a279514 100644
@@ -116,7 +116,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)

@@ -142,7 +142,7 @@ static bool mem_overlaps(struct mem_vector *one, struct mem_vector *two)
 }
 
 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)
 {
        u64 initrd_start, initrd_size;
        u64 cmd_line, cmd_line_size;

@@ -151,10 +151,13 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
 
        /*
         * Avoid the region that is unsafe to overlap during
-        * decompression (see calculations at top of misc.c).
+        * decompression.
+        * As we already move ZO (arch/x86/boot/compressed/vmlinux)
+        * to the end of buffer, [input+input_size, output+init_size)
+        * has [_text, _end) for ZO.
         */
-       unsafe_len = (output_size >> 12) + 32768 + 18;
-       unsafe = (unsigned long)input + input_size - unsafe_len;
+       unsafe_len = output + init_size - (input + input_size);
+       unsafe = (unsigned long)input + input_size;
        mem_avoid[0].start = unsafe;
        mem_avoid[0].size = unsafe_len;
 

@@ -176,13 +179,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? */

@@ -327,7 +326,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
                                      unsigned char *input,
                                      unsigned long input_size,
                                      unsigned char *output,
-                                     unsigned long output_size)
+                                     unsigned long init_size)
 {
        unsigned long choice = (unsigned long)output;
        unsigned long random;

@@ -349,10 +348,10 @@ unsigned char *choose_kernel_location(struct boot_params *params,
 
        /* Record the various known unsafe memory ranges. */
        mem_avoid_init((unsigned long)input, input_size,
-                      (unsigned long)output, output_size);
+                      (unsigned long)output, init_size);
 
        /* Walk e820 and find a random address. */
-       random = find_random_addr(choice, output_size);
+       random = find_random_addr(choice, init_size);
        if (!random) {
                debug_putstr("KASLR could not find suitable E820 region...\n");
                goto out;

arch/x86/boot/compressed/misc.h +2 -3

index ee3576b..23156e7 100644
@@ -61,7 +61,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
                                      unsigned char *input,
                                      unsigned long input_size,
                                      unsigned char *output,
-                                     unsigned long output_size);
+                                     unsigned long init_size);
 /* cpuflags.c */
 bool has_cpuflag(int flag);
 #else

@@ -70,7 +70,7 @@ unsigned char *choose_kernel_location(struct boot_params *params,
                                      unsigned char *input,
                                      unsigned long input_size,
                                      unsigned char *output,
-                                     unsigned long output_size)
+                                     unsigned long init_size)
 {
        return output;
 }
--

1.8.4.5

On Sat, Mar 7, 2015 at 2:07 PM, Yinghai Lu <[email protected]> wrote:

This may be a stupid question, but are boot_params being used outside of the compressed loader? If so, it might make sense to split that change into a separate patch to go to stable, if it's causing problems. (And document what problem is getting solved.)

Quoted 6 lines · Kees Cook

On Sat, Mar 7, 2015 at 2:07 PM, Yinghai Lu <[email protected]> wrote:

This may be a stupid question, but are boot_params being used outside of the compressed loader? If so, it might make sense to split that change into a separate patch to go to stable, if it's causing problems. (And document what problem is getting solved.)

boot_params will keep the same and until it is passed x86_64_start_kernel in vmlinux.
and there it will be copied, same as cmdline.

but current kaslr only support random the base high, and it does not support kexec
and current boot loader (grub2) put it really low. (under 1M). the real real_mode. :).

Thanks

Yinghai

We need to include that in boot::decompress_kernel stage to set new ident mapping.

Also add checking for __pa/__va macro definition, as we need to override them in boot::decompress_kernel stage.

Signed-off-by Yinghai Lu <[email protected]>

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

arch/x86/include/asm/page.h +5 -0

index 802dde3..cf8f619 100644
@@ -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)

arch/x86/mm/ident_map.c new file +74 -0

new file mode 100644
index 0000000..751ca92
@@ -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;
+}

arch/x86/mm/init_64.c +1 -74

index 30eb05a..c30efb6 100644
@@ -56,79 +56,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"
 
 static int __init parse_direct_gbpages_off(char *arg)
 {
--

1.8.4.5

First, aslr will support to put random VO above 4G, so we must set ident mapping for the range even we come from startup_32 path.

Second, when boot from 64bit bootloader, bootloader set ident mapping, and boot via ZO (arch/x86/boot/compressed/vmlinux) startup_64. Those pages for pagetable need to be avoided when we select new random VO (vmlinux) base. Otherwise decompressor would overwrite them during decompressing.

One solution: go through pagetable and find out every page is used by pagetable for every mem_aovid checking but we will need extra code.

Other solution: create new ident mapping instead, and pages for pagetable will sit in _pagetable section of ZO, and they are in mem_avoid array already. In this way, we can reuse the code for setting ident mapping.

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

Need to increase pgt buffer size.
When boot via startup_64, as we need to cover old VO, params, cmdline and new VO, in extreme case we could have them all cross 512G boundary, will need (2+2)*4 pages with 2M mapping. And need 2 for first 2M for vga ram. Plus one for level4. Total will be 19 pages.
When boot via startup_32, aslr would move new VO above 4G, we need set extra ident mapping for new VO, pgt buffer come from _pgtable offset 6 pages. should only need (2+2) pages at most when it cross 512G boundary. So 19 pages could make both pathes happy.

-v3: add mapping for first 2M with video ram when X86_VERBOSE_BOOTUP is set.
Don't need to set mapping for setup_data, as it is already late in boot/ZO stage, will not access it until VO stage, and VO stage will use early_memmap or kernel address to access them.

Cc Kees Cook <[email protected]>

Cc Jiri Kosina <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Matt Fleming <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/boot/compressed/aslr.c     | 21 ++++++++
 arch/x86/boot/compressed/head_64.S  |  4 +-
 arch/x86/boot/compressed/misc_pgt.c | 98 +++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/boot.h         | 19 +++++++
 4 files changed, 140 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/boot/compressed/misc_pgt.c

arch/x86/boot/compressed/aslr.c +5 -0

index a279514..34eb652 100644
@@ -1,3 +1,8 @@
+#ifdef CONFIG_X86_64
+#define __pa(x)  ((unsigned long)(x))
+#define __va(x)  ((void *)((unsigned long)(x)))
+#endif
+
 #include "misc.h"
 
 #include <asm/msr.h>

arch/x86/boot/compressed/aslr.c +15 -0

+
 #define I8254_PORT_CONTROL     0x43
 #define I8254_PORT_COUNTER0    0x40
 #define I8254_CMD_READBACK     0xC0

@@ -160,6 +167,7 @@ static void mem_avoid_init(unsigned long input, unsigned long input_size,
        unsafe = (unsigned long)input + input_size;
        mem_avoid[0].start = unsafe;
        mem_avoid[0].size = unsafe_len;
+       fill_pagetable(output, init_size);
 
        /* Avoid initrd. */
        initrd_start  = (u64)real_mode->ext_ramdisk_image << 32;

@@ -168,6 +176,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;
+       /* don't need to set mapping for initrd */
 
        /* Avoid kernel command line. */
        cmd_line  = (u64)real_mode->ext_cmd_line_ptr << 32;

@@ -178,10 +187,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(cmd_line, cmd_line_size);
 
        /* Avoid params */
        mem_avoid[3].start = (unsigned long)real_mode;
        mem_avoid[3].size = sizeof(*real_mode);
+       fill_pagetable((unsigned long)real_mode, sizeof(*real_mode));
+
+       /* 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? */

@@ -362,6 +380,9 @@ unsigned char *choose_kernel_location(struct boot_params *params,
                goto out;
 
        choice = random;
+
+       fill_pagetable(choice, init_size);
+       switch_pagetable();
 out:
        return (unsigned char *)choice;
 }

arch/x86/boot/compressed/head_64.S +2 -2

index 69015b5..1b6e34a 100644
@@ -125,7 +125,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 */

@@ -477,4 +477,4 @@ boot_stack_end:
        .section ".pgtable","a",@nobits
        .balign 4096
 pgtable:
-       .fill 6*4096, 1, 0
+       .fill BOOT_PGT_SIZE, 1, 0

arch/x86/boot/compressed/misc_pgt.c new file +98 -0

new file mode 100644
index 0000000..b55982c
@@ -0,0 +1,98 @@
+
+#ifdef CONFIG_X86_64
+#include <asm/init.h>
+#include <asm/pgtable.h>
+
+#include "../../mm/ident_map.c"
+
+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");
+               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
+#define memzero(s, n)   memset((s), 0, (n))
+
+unsigned long __force_order;
+static struct alloc_pgt_data pgt_data;
+static struct x86_mapping_info mapping_info;
+static pgd_t *level4p;
+
+extern unsigned char _pgtable[];
+static 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;
+
+                       debug_putstr("boot via startup_32\n");
+               } else {
+                       pgt_data.pgt_buf = (unsigned char *)_pgtable;
+                       pgt_data.pgt_buf_size = BOOT_PGT_SIZE;
+
+                       debug_putstr("boot via startup_64\n");
+                       level4p = (pgd_t *)alloc_pgt_page(&pgt_data);
+               }
+               memset((unsigned char *)pgt_data.pgt_buf, 0,
+                        pgt_data.pgt_buf_size);
+       }
+
+       /* 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);
+}
+
+static void switch_pagetable(void)
+{
+       write_cr3((unsigned long)level4p);
+}
+
+#else
+static void fill_pagetable(unsigned long start, unsigned long size)
+{
+}
+static void switch_pagetable(void)
+{
+}
+#endif

arch/x86/include/asm/boot.h +19 -1

index 4fa687a..7b23908 100644
@@ -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
--

1.8.4.5

Quoted 65 lines · Yinghai Lu

First, aslr will support to put random VO above 4G, so we must set ident mapping for the range even we come from startup_32 path.

Second, when boot from 64bit bootloader, bootloader set ident mapping, and boot via ZO (arch/x86/boot/compressed/vmlinux) startup_64. Those pages for pagetable need to be avoided when we select new random VO (vmlinux) base. Otherwise decompressor would overwrite them during decompressing.

One solution: go through pagetable and find out every page is used by pagetable for every mem_aovid checking but we will need extra code.

Other solution: create new ident mapping instead, and pages for pagetable will sit in _pagetable section of ZO, and they are in mem_avoid array already. In this way, we can reuse the code for setting ident mapping.

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

Need to increase pgt buffer size.
When boot via startup_64, as we need to cover old VO, params, cmdline and new VO, in extreme case we could have them all cross 512G boundary, will need (2+2)*4 pages with 2M mapping. And need 2 for first 2M for vga ram. Plus one for level4. Total will be 19 pages.
When boot via startup_32, aslr would move new VO above 4G, we need set extra ident mapping for new VO, pgt buffer come from _pgtable offset 6 pages. should only need (2+2) pages at most when it cross 512G boundary. So 19 pages could make both pathes happy.

-v3: add mapping for first 2M with video ram when X86_VERBOSE_BOOTUP is set.
Don't need to set mapping for setup_data, as it is already late in boot/ZO stage, will not access it until VO stage, and VO stage will use early_memmap or kernel address to access them.

Cc Kees Cook <[email protected]>

Cc Jiri Kosina <[email protected]>

Cc Borislav Petkov <[email protected]>

Cc Matt Fleming <[email protected]>

Signed-off-by Yinghai Lu <[email protected]>

---
 arch/x86/boot/compressed/aslr.c     | 21 ++++++++
 arch/x86/boot/compressed/head_64.S  |  4 +-
 arch/x86/boot/compressed/misc_pgt.c | 98 +++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/boot.h         | 19 +++++++
 4 files changed, 140 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/boot/compressed/misc_pgt.c

arch/x86/boot/compressed/aslr.c +5 -0

index a279514..34eb652 100644
@@ -1,3 +1,8 @@
+#ifdef CONFIG_X86_64
+#define __pa(x)  ((unsigned long)(x))
+#define __va(x)  ((void *)((unsigned long)(x)))
+#endif
+
 #include "misc.h"
 
 #include <asm/msr.h>

@@ -21,6 +26,8 @@ struct kaslr_setup_data {
        __u8 data[1];
 } kaslr_setup_data;
 

arch/x86/boot/compressed/aslr.c struct kaslr_setup_data { · line 29

Quoted 1 line

arch/x86/boot/compressed/aslr.c +1 -0

+#include "misc_pgt.c"

Shouldn't this just be a normal built .o file that is linked together in the Makefile, specifically tracking CONFIG_RANDOMIZE_BASE as aslr.o already is?

Quoted 6 lines

Quoted 2 lines

+#include "misc_pgt.c"

Shouldn't this just be a normal built .o file that is linked together in the Makefile, specifically tracking CONFIG_RANDOMIZE_BASE as aslr.o already is?

Yes, we could go that way.