linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
@ 2014-10-31 13:40 Junjie Mao
  2014-10-31 16:11 ` Kees Cook
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Junjie Mao @ 2014-10-31 13:40 UTC (permalink / raw)
  To: Kees Cook
  Cc: Junjie Mao, linux-kernel, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, x86, Josh Triplett, Matt Fleming, Ard Biesheuvel,
	Vivek Goyal, Andi Kleen, Fengguang Wu, stable

When choosing a random address, the current implementation does not take into
account the reversed space for .bss and .brk sections. Thus the relocated kernel
may overlap other components in memory. Here is an example of the overlap from a
x86_64 kernel in qemu (the ranges of physical addresses are presented):

 Physical Address

    0x0fe00000                  --+--------------------+  <-- randomized base
                               /  |  relocated kernel  |
                   vmlinux.bin    | (from vmlinux.bin) |
    0x1336d000    (an ELF file)   +--------------------+--
                               \  |                    |  \
    0x1376d870                  --+--------------------+   |
                                  |    relocs table    |   |
    0x13c1c2a8                    +--------------------+   .bss and .brk
                                  |                    |   |
    0x13ce6000                    +--------------------+   |
                                  |                    |  /
    0x13f77000                    |       initrd       |--
                                  |                    |
    0x13fef374                    +--------------------+

The initrd image will then be overwritten by the memset during early
initialization:

[    1.655204] Unpacking initramfs...
[    1.662831] Initramfs unpacking failed: junk in compressed archive

This patch prevents the above situation by requiring a larger space when looking
for a random kernel base, so that existing logic can effectively avoids the
overlap.

Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps")
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Junjie Mao <eternal.n08@gmail.com>
[kees: switched to perl to avoid hex translation pain in mawk vs gawk]
[kees: calculated overlap without relocs table]
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: stable@vger.kernel.org
---
This version updates the commit log only.

Kees, please help review the documentation. Thanks!

Best Regards
Junjie Mao
---
 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    | 13 ++++++++++---
 arch/x86/boot/compressed/mkpiggy.c |  9 +++++++--
 arch/x86/tools/calc_run_size.pl    | 30 ++++++++++++++++++++++++++++++
 6 files changed, 57 insertions(+), 9 deletions(-)
 create mode 100644 arch/x86/tools/calc_run_size.pl

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 0fcd9133790c..14fe7cba21d1 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -75,8 +75,10 @@ suffix-$(CONFIG_KERNEL_XZ)	:= xz
 suffix-$(CONFIG_KERNEL_LZO) 	:= lzo
 suffix-$(CONFIG_KERNEL_LZ4) 	:= lz4

+RUN_SIZE = $(shell objdump -h vmlinux | \
+	     perl $(srctree)/arch/x86/tools/calc_run_size.pl)
 quiet_cmd_mkpiggy = MKPIGGY $@
-      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
+      cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )

 targets += piggy.S
 $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index cbed1407a5cd..1d7fbbcc196d 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -207,7 +207,8 @@ relocated:
  * Do the decompression, and jump to the new kernel..
  */
 				/* push arguments for decompress_kernel: */
-	pushl	$z_output_len	/* decompressed length */
+	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
 	pushl	%ebp		/* output address */
 	pushl	$z_input_len	/* input_len */
@@ -217,7 +218,7 @@ relocated:
 	pushl	%eax		/* heap area */
 	pushl	%esi		/* real mode pointer */
 	call	decompress_kernel /* returns kernel location in %eax */
-	addl	$24, %esp
+	addl	$28, %esp

 /*
  * Jump to the decompressed kernel.
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 2884e0c3e8a5..6b1766c6c082 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -402,13 +402,16 @@ 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 */
+	movq	$z_output_len, %r9	/* decompressed length, end of relocs */
 	call	decompress_kernel	/* returns kernel location in %rax */
+	popq	%r9
 	popq	%rsi

 /*
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 57ab74df7eea..30dd59a9f0b4 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -358,7 +358,8 @@ 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 output_len,
+				  unsigned long run_size)
 {
 	real_mode = rmode;

@@ -381,8 +382,14 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	free_mem_ptr     = heap;	/* Heap */
 	free_mem_end_ptr = heap + BOOT_HEAP_SIZE;

-	output = choose_kernel_location(input_data, input_len,
-					output, output_len);
+	/*
+	 * 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.
+	 */
+	output = choose_kernel_location(input_data, input_len, output,
+					output_len > run_size ? output_len
+							      : run_size);

 	/* Validate memory location choices. */
 	if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index b669ab65bf6c..d8222f213182 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -36,11 +36,13 @@ 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 < 2) {
-		fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
+	if (argc < 3) {
+		fprintf(stderr, "Usage: %s compressed_file run_size\n",
+				argv[0]);
 		goto bail;
 	}

@@ -74,6 +76,7 @@ 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");
@@ -85,6 +88,8 @@ 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");
diff --git a/arch/x86/tools/calc_run_size.pl b/arch/x86/tools/calc_run_size.pl
new file mode 100644
index 000000000000..0b0b124d3ece
--- /dev/null
+++ b/arch/x86/tools/calc_run_size.pl
@@ -0,0 +1,30 @@
+#!/usr/bin/perl
+#
+# Calculate the amount of space needed to run the kernel, including room for
+# the .bss and .brk sections.
+#
+# Usage:
+# objdump -h a.out | perl calc_run_size.pl
+use strict;
+
+my $mem_size = 0;
+my $file_offset = 0;
+
+my $sections=" *[0-9]+ \.(?:bss|brk) +";
+while (<>) {
+	if (/^$sections([0-9a-f]+) +(?:[0-9a-f]+ +){2}([0-9a-f]+)/) {
+		my $size = hex($1);
+		my $offset = hex($2);
+		$mem_size += $size;
+		if ($file_offset == 0) {
+			$file_offset = $offset;
+		} elsif ($file_offset != $offset) {
+			die ".bss and .brk lack common file offset\n";
+		}
+	}
+}
+
+if ($file_offset == 0) {
+	die "Never found .bss or .brk file offset\n";
+}
+printf("%d\n", $mem_size + $file_offset);
--
1.9.3

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

* Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
  2014-10-31 13:40 [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd Junjie Mao
@ 2014-10-31 16:11 ` Kees Cook
  2014-11-01 21:24 ` [tip:x86/urgent] " tip-bot for Junjie Mao
  2014-11-18  2:17 ` [PATCH v3] " Greg Thelen
  2 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2014-10-31 16:11 UTC (permalink / raw)
  To: Junjie Mao, H. Peter Anvin
  Cc: LKML, Thomas Gleixner, Ingo Molnar, x86, Josh Triplett,
	Matt Fleming, Ard Biesheuvel, Vivek Goyal, Andi Kleen,
	Fengguang Wu

On Fri, Oct 31, 2014 at 6:40 AM, Junjie Mao <eternal.n08@gmail.com> wrote:
> When choosing a random address, the current implementation does not take into
> account the reversed space for .bss and .brk sections. Thus the relocated kernel
> may overlap other components in memory. Here is an example of the overlap from a
> x86_64 kernel in qemu (the ranges of physical addresses are presented):
>
>  Physical Address
>
>     0x0fe00000                  --+--------------------+  <-- randomized base
>                                /  |  relocated kernel  |
>                    vmlinux.bin    | (from vmlinux.bin) |
>     0x1336d000    (an ELF file)   +--------------------+--
>                                \  |                    |  \
>     0x1376d870                  --+--------------------+   |
>                                   |    relocs table    |   |
>     0x13c1c2a8                    +--------------------+   .bss and .brk
>                                   |                    |   |
>     0x13ce6000                    +--------------------+   |
>                                   |                    |  /
>     0x13f77000                    |       initrd       |--
>                                   |                    |
>     0x13fef374                    +--------------------+
>
> The initrd image will then be overwritten by the memset during early
> initialization:
>
> [    1.655204] Unpacking initramfs...
> [    1.662831] Initramfs unpacking failed: junk in compressed archive
>
> This patch prevents the above situation by requiring a larger space when looking
> for a random kernel base, so that existing logic can effectively avoids the
> overlap.
>
> Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps")
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Junjie Mao <eternal.n08@gmail.com>
> [kees: switched to perl to avoid hex translation pain in mawk vs gawk]
> [kees: calculated overlap without relocs table]
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> ---
> This version updates the commit log only.
>
> Kees, please help review the documentation. Thanks!

Looks great, thanks! Can someone from x86 pull this please?

-Kees

>
> Best Regards
> Junjie Mao
> ---
>  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    | 13 ++++++++++---
>  arch/x86/boot/compressed/mkpiggy.c |  9 +++++++--
>  arch/x86/tools/calc_run_size.pl    | 30 ++++++++++++++++++++++++++++++
>  6 files changed, 57 insertions(+), 9 deletions(-)
>  create mode 100644 arch/x86/tools/calc_run_size.pl
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index 0fcd9133790c..14fe7cba21d1 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -75,8 +75,10 @@ suffix-$(CONFIG_KERNEL_XZ)   := xz
>  suffix-$(CONFIG_KERNEL_LZO)    := lzo
>  suffix-$(CONFIG_KERNEL_LZ4)    := lz4
>
> +RUN_SIZE = $(shell objdump -h vmlinux | \
> +            perl $(srctree)/arch/x86/tools/calc_run_size.pl)
>  quiet_cmd_mkpiggy = MKPIGGY $@
> -      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
> +      cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
>
>  targets += piggy.S
>  $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
> diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
> index cbed1407a5cd..1d7fbbcc196d 100644
> --- a/arch/x86/boot/compressed/head_32.S
> +++ b/arch/x86/boot/compressed/head_32.S
> @@ -207,7 +207,8 @@ relocated:
>   * Do the decompression, and jump to the new kernel..
>   */
>                                 /* push arguments for decompress_kernel: */
> -       pushl   $z_output_len   /* decompressed length */
> +       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
>         pushl   %ebp            /* output address */
>         pushl   $z_input_len    /* input_len */
> @@ -217,7 +218,7 @@ relocated:
>         pushl   %eax            /* heap area */
>         pushl   %esi            /* real mode pointer */
>         call    decompress_kernel /* returns kernel location in %eax */
> -       addl    $24, %esp
> +       addl    $28, %esp
>
>  /*
>   * Jump to the decompressed kernel.
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 2884e0c3e8a5..6b1766c6c082 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -402,13 +402,16 @@ 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 */
> +       movq    $z_output_len, %r9      /* decompressed length, end of relocs */
>         call    decompress_kernel       /* returns kernel location in %rax */
> +       popq    %r9
>         popq    %rsi
>
>  /*
> diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
> index 57ab74df7eea..30dd59a9f0b4 100644
> --- a/arch/x86/boot/compressed/misc.c
> +++ b/arch/x86/boot/compressed/misc.c
> @@ -358,7 +358,8 @@ 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 output_len,
> +                                 unsigned long run_size)
>  {
>         real_mode = rmode;
>
> @@ -381,8 +382,14 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
>         free_mem_ptr     = heap;        /* Heap */
>         free_mem_end_ptr = heap + BOOT_HEAP_SIZE;
>
> -       output = choose_kernel_location(input_data, input_len,
> -                                       output, output_len);
> +       /*
> +        * 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.
> +        */
> +       output = choose_kernel_location(input_data, input_len, output,
> +                                       output_len > run_size ? output_len
> +                                                             : run_size);
>
>         /* Validate memory location choices. */
>         if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
> diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
> index b669ab65bf6c..d8222f213182 100644
> --- a/arch/x86/boot/compressed/mkpiggy.c
> +++ b/arch/x86/boot/compressed/mkpiggy.c
> @@ -36,11 +36,13 @@ 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 < 2) {
> -               fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
> +       if (argc < 3) {
> +               fprintf(stderr, "Usage: %s compressed_file run_size\n",
> +                               argv[0]);
>                 goto bail;
>         }
>
> @@ -74,6 +76,7 @@ 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");
> @@ -85,6 +88,8 @@ 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");
> diff --git a/arch/x86/tools/calc_run_size.pl b/arch/x86/tools/calc_run_size.pl
> new file mode 100644
> index 000000000000..0b0b124d3ece
> --- /dev/null
> +++ b/arch/x86/tools/calc_run_size.pl
> @@ -0,0 +1,30 @@
> +#!/usr/bin/perl
> +#
> +# Calculate the amount of space needed to run the kernel, including room for
> +# the .bss and .brk sections.
> +#
> +# Usage:
> +# objdump -h a.out | perl calc_run_size.pl
> +use strict;
> +
> +my $mem_size = 0;
> +my $file_offset = 0;
> +
> +my $sections=" *[0-9]+ \.(?:bss|brk) +";
> +while (<>) {
> +       if (/^$sections([0-9a-f]+) +(?:[0-9a-f]+ +){2}([0-9a-f]+)/) {
> +               my $size = hex($1);
> +               my $offset = hex($2);
> +               $mem_size += $size;
> +               if ($file_offset == 0) {
> +                       $file_offset = $offset;
> +               } elsif ($file_offset != $offset) {
> +                       die ".bss and .brk lack common file offset\n";
> +               }
> +       }
> +}
> +
> +if ($file_offset == 0) {
> +       die "Never found .bss or .brk file offset\n";
> +}
> +printf("%d\n", $mem_size + $file_offset);
> --
> 1.9.3



-- 
Kees Cook
Chrome OS Security

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

* [tip:x86/urgent] x86, kaslr: Prevent .bss from overlaping initrd
  2014-10-31 13:40 [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd Junjie Mao
  2014-10-31 16:11 ` Kees Cook
@ 2014-11-01 21:24 ` tip-bot for Junjie Mao
  2014-11-18  2:17 ` [PATCH v3] " Greg Thelen
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Junjie Mao @ 2014-11-01 21:24 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: keescook, vgoyal, mingo, tglx, hpa, matt.fleming, ard.biesheuvel,
	josh, eternal.n08, fengguang.wu, ak, linux-kernel

Commit-ID:  e6023367d779060fddc9a52d1f474085b2b36298
Gitweb:     http://git.kernel.org/tip/e6023367d779060fddc9a52d1f474085b2b36298
Author:     Junjie Mao <eternal.n08@gmail.com>
AuthorDate: Fri, 31 Oct 2014 21:40:38 +0800
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Sat, 1 Nov 2014 22:20:50 +0100

x86, kaslr: Prevent .bss from overlaping initrd

When choosing a random address, the current implementation does not take into
account the reversed space for .bss and .brk sections. Thus the relocated kernel
may overlap other components in memory. Here is an example of the overlap from a
x86_64 kernel in qemu (the ranges of physical addresses are presented):

 Physical Address

    0x0fe00000                  --+--------------------+  <-- randomized base
                               /  |  relocated kernel  |
                   vmlinux.bin    | (from vmlinux.bin) |
    0x1336d000    (an ELF file)   +--------------------+--
                               \  |                    |  \
    0x1376d870                  --+--------------------+   |
                                  |    relocs table    |   |
    0x13c1c2a8                    +--------------------+   .bss and .brk
                                  |                    |   |
    0x13ce6000                    +--------------------+   |
                                  |                    |  /
    0x13f77000                    |       initrd       |--
                                  |                    |
    0x13fef374                    +--------------------+

The initrd image will then be overwritten by the memset during early
initialization:

[    1.655204] Unpacking initramfs...
[    1.662831] Initramfs unpacking failed: junk in compressed archive

This patch prevents the above situation by requiring a larger space when looking
for a random kernel base, so that existing logic can effectively avoids the
overlap.

[kees: switched to perl to avoid hex translation pain in mawk vs gawk]
[kees: calculated overlap without relocs table]

Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps")
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Junjie Mao <eternal.n08@gmail.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
Cc: Josh Triplett <josh@joshtriplett.org>
Cc: Matt Fleming <matt.fleming@intel.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Vivek Goyal <vgoyal@redhat.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: stable@vger.kernel.org
Link: http://lkml.kernel.org/r/1414762838-13067-1-git-send-email-eternal.n08@gmail.com
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 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    | 13 ++++++++++---
 arch/x86/boot/compressed/mkpiggy.c |  9 +++++++--
 arch/x86/tools/calc_run_size.pl    | 30 ++++++++++++++++++++++++++++++
 6 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index 704f58a..be1e07d 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -76,8 +76,10 @@ suffix-$(CONFIG_KERNEL_XZ)	:= xz
 suffix-$(CONFIG_KERNEL_LZO) 	:= lzo
 suffix-$(CONFIG_KERNEL_LZ4) 	:= lz4
 
+RUN_SIZE = $(shell objdump -h vmlinux | \
+	     perl $(srctree)/arch/x86/tools/calc_run_size.pl)
 quiet_cmd_mkpiggy = MKPIGGY $@
-      cmd_mkpiggy = $(obj)/mkpiggy $< > $@ || ( rm -f $@ ; false )
+      cmd_mkpiggy = $(obj)/mkpiggy $< $(RUN_SIZE) > $@ || ( rm -f $@ ; false )
 
 targets += piggy.S
 $(obj)/piggy.S: $(obj)/vmlinux.bin.$(suffix-y) $(obj)/mkpiggy FORCE
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index cbed140..1d7fbbc 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -207,7 +207,8 @@ relocated:
  * Do the decompression, and jump to the new kernel..
  */
 				/* push arguments for decompress_kernel: */
-	pushl	$z_output_len	/* decompressed length */
+	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
 	pushl	%ebp		/* output address */
 	pushl	$z_input_len	/* input_len */
@@ -217,7 +218,7 @@ relocated:
 	pushl	%eax		/* heap area */
 	pushl	%esi		/* real mode pointer */
 	call	decompress_kernel /* returns kernel location in %eax */
-	addl	$24, %esp
+	addl	$28, %esp
 
 /*
  * Jump to the decompressed kernel.
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 2884e0c..6b1766c 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -402,13 +402,16 @@ 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 */
+	movq	$z_output_len, %r9	/* decompressed length, end of relocs */
 	call	decompress_kernel	/* returns kernel location in %rax */
+	popq	%r9
 	popq	%rsi
 
 /*
diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c
index 57ab74d..30dd59a 100644
--- a/arch/x86/boot/compressed/misc.c
+++ b/arch/x86/boot/compressed/misc.c
@@ -358,7 +358,8 @@ 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 output_len,
+				  unsigned long run_size)
 {
 	real_mode = rmode;
 
@@ -381,8 +382,14 @@ asmlinkage __visible void *decompress_kernel(void *rmode, memptr heap,
 	free_mem_ptr     = heap;	/* Heap */
 	free_mem_end_ptr = heap + BOOT_HEAP_SIZE;
 
-	output = choose_kernel_location(input_data, input_len,
-					output, output_len);
+	/*
+	 * 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.
+	 */
+	output = choose_kernel_location(input_data, input_len, output,
+					output_len > run_size ? output_len
+							      : run_size);
 
 	/* Validate memory location choices. */
 	if ((unsigned long)output & (MIN_KERNEL_ALIGN - 1))
diff --git a/arch/x86/boot/compressed/mkpiggy.c b/arch/x86/boot/compressed/mkpiggy.c
index b669ab6..d8222f2 100644
--- a/arch/x86/boot/compressed/mkpiggy.c
+++ b/arch/x86/boot/compressed/mkpiggy.c
@@ -36,11 +36,13 @@ 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 < 2) {
-		fprintf(stderr, "Usage: %s compressed_file\n", argv[0]);
+	if (argc < 3) {
+		fprintf(stderr, "Usage: %s compressed_file run_size\n",
+				argv[0]);
 		goto bail;
 	}
 
@@ -74,6 +76,7 @@ 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");
@@ -85,6 +88,8 @@ 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");
diff --git a/arch/x86/tools/calc_run_size.pl b/arch/x86/tools/calc_run_size.pl
new file mode 100644
index 0000000..0b0b124
--- /dev/null
+++ b/arch/x86/tools/calc_run_size.pl
@@ -0,0 +1,30 @@
+#!/usr/bin/perl
+#
+# Calculate the amount of space needed to run the kernel, including room for
+# the .bss and .brk sections.
+#
+# Usage:
+# objdump -h a.out | perl calc_run_size.pl
+use strict;
+
+my $mem_size = 0;
+my $file_offset = 0;
+
+my $sections=" *[0-9]+ \.(?:bss|brk) +";
+while (<>) {
+	if (/^$sections([0-9a-f]+) +(?:[0-9a-f]+ +){2}([0-9a-f]+)/) {
+		my $size = hex($1);
+		my $offset = hex($2);
+		$mem_size += $size;
+		if ($file_offset == 0) {
+			$file_offset = $offset;
+		} elsif ($file_offset != $offset) {
+			die ".bss and .brk lack common file offset\n";
+		}
+	}
+}
+
+if ($file_offset == 0) {
+	die "Never found .bss or .brk file offset\n";
+}
+printf("%d\n", $mem_size + $file_offset);

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

* Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
  2014-10-31 13:40 [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd Junjie Mao
  2014-10-31 16:11 ` Kees Cook
  2014-11-01 21:24 ` [tip:x86/urgent] " tip-bot for Junjie Mao
@ 2014-11-18  2:17 ` Greg Thelen
  2014-11-18  2:23   ` Greg Thelen
  2 siblings, 1 reply; 6+ messages in thread
From: Greg Thelen @ 2014-11-18  2:17 UTC (permalink / raw)
  To: Junjie Mao
  Cc: Kees Cook, linux-kernel, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, x86, Josh Triplett, Matt Fleming, Ard Biesheuvel,
	Vivek Goyal, Andi Kleen, Fengguang Wu, stable

On Fri, Oct 31 2014, Junjie Mao wrote:

> When choosing a random address, the current implementation does not take into
> account the reversed space for .bss and .brk sections. Thus the relocated kernel
> may overlap other components in memory. Here is an example of the overlap from a
> x86_64 kernel in qemu (the ranges of physical addresses are presented):
>
>  Physical Address
>
>     0x0fe00000                  --+--------------------+  <-- randomized base
>                                /  |  relocated kernel  |
>                    vmlinux.bin    | (from vmlinux.bin) |
>     0x1336d000    (an ELF file)   +--------------------+--
>                                \  |                    |  \
>     0x1376d870                  --+--------------------+   |
>                                   |    relocs table    |   |
>     0x13c1c2a8                    +--------------------+   .bss and .brk
>                                   |                    |   |
>     0x13ce6000                    +--------------------+   |
>                                   |                    |  /
>     0x13f77000                    |       initrd       |--
>                                   |                    |
>     0x13fef374                    +--------------------+
>
> The initrd image will then be overwritten by the memset during early
> initialization:
>
> [    1.655204] Unpacking initramfs...
> [    1.662831] Initramfs unpacking failed: junk in compressed archive
>
> This patch prevents the above situation by requiring a larger space when looking
> for a random kernel base, so that existing logic can effectively avoids the
> overlap.
>
> Fixes: 82fa9637a2 ("x86, kaslr: Select random position from e820 maps")
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Junjie Mao <eternal.n08@gmail.com>
> [kees: switched to perl to avoid hex translation pain in mawk vs gawk]
> [kees: calculated overlap without relocs table]
> Signed-off-by: Kees Cook <keescook@chromium.org>
> Cc: stable@vger.kernel.org
> ---
> This version updates the commit log only.
>
> Kees, please help review the documentation. Thanks!
>
> Best Regards
> Junjie Mao
[...]
> diff --git a/arch/x86/tools/calc_run_size.pl b/arch/x86/tools/calc_run_size.pl
> new file mode 100644
> index 000000000000..0b0b124d3ece
> --- /dev/null
> +++ b/arch/x86/tools/calc_run_size.pl
> @@ -0,0 +1,30 @@
> +#!/usr/bin/perl
> +#
> +# Calculate the amount of space needed to run the kernel, including room for
> +# the .bss and .brk sections.
> +#
> +# Usage:
> +# objdump -h a.out | perl calc_run_size.pl
> +use strict;
> +
> +my $mem_size = 0;
> +my $file_offset = 0;
> +
> +my $sections=" *[0-9]+ \.(?:bss|brk) +";
> +while (<>) {
> + if (/^$sections([0-9a-f]+) +(?:[0-9a-f]+ +){2}([0-9a-f]+)/) {
> + my $size = hex($1);
> + my $offset = hex($2);
> + $mem_size += $size;
> + if ($file_offset == 0) {
> + $file_offset = $offset;
> + } elsif ($file_offset != $offset) {
> + die ".bss and .brk lack common file offset\n";
> + }
> + }
> +}
> +
> +if ($file_offset == 0) {
> + die "Never found .bss or .brk file offset\n";
> +}
> +printf("%d\n", $mem_size + $file_offset);

Given that bss and brk are nobits (i.e. only ALLOC) sections, does
file_offset make sense as a load address.  This fails with gold:

$ git checkout v3.18-rc5
$ make  # with gold
[...]
..bss and .brk lack common file offset
..bss and .brk lack common file offset
..bss and .brk lack common file offset
..bss and .brk lack common file offset
  MKPIGGY arch/x86/boot/compressed/piggy.S
Usage: arch/x86/boot/compressed/mkpiggy compressed_file run_size
make[2]: *** [arch/x86/boot/compressed/piggy.S] Error 1
make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2
make: *** [bzImage] Error 2

In ld.bfd brk/bss file_offsets match, but they differ with ld.gold:

$ objdump -h vmlinux.ld
[...]
  0 .text         00818bb3  ffffffff81000000  0000000001000000  00200000  2**12
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss          000e0000  ffffffff81fe8000  0000000001fe8000  013e8000  2**12
                  ALLOC
 27 .brk          00026000  ffffffff820c8000  00000000020c8000  013e8000  2**0
                  ALLOC

$ objdump -h vmlinux.ld | perl arch/x86/tools/calc_run_size.pl
21946368
# aka 0x14ee000


$ objdump -h vmlinux.gold
[...]
  0 .text         00818bb3  ffffffff81000000  0000000001000000  00001000  2**12
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss          000e0000  ffffffff81feb000  0000000001feb000  00e90000  2**12
                  ALLOC
 27 .brk          00026000  ffffffff820cb000  00000000020cb000  00f70000  2**0
                  ALLOC

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

* Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
  2014-11-18  2:17 ` [PATCH v3] " Greg Thelen
@ 2014-11-18  2:23   ` Greg Thelen
  2014-11-19 16:26     ` H. Peter Anvin
  0 siblings, 1 reply; 6+ messages in thread
From: Greg Thelen @ 2014-11-18  2:23 UTC (permalink / raw)
  To: Junjie Mao
  Cc: Kees Cook, linux-kernel, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, x86, Josh Triplett, Matt Fleming, Ard Biesheuvel,
	Vivek Goyal, Andi Kleen, Fengguang Wu, stable


On Mon, Nov 17 2014, Greg Thelen wrote:
[...]
> Given that bss and brk are nobits (i.e. only ALLOC) sections, does
> file_offset make sense as a load address.  This fails with gold:
>
> $ git checkout v3.18-rc5
> $ make  # with gold
> [...]
> ..bss and .brk lack common file offset
> ..bss and .brk lack common file offset
> ..bss and .brk lack common file offset
> ..bss and .brk lack common file offset
>   MKPIGGY arch/x86/boot/compressed/piggy.S
> Usage: arch/x86/boot/compressed/mkpiggy compressed_file run_size
> make[2]: *** [arch/x86/boot/compressed/piggy.S] Error 1
> make[1]: *** [arch/x86/boot/compressed/vmlinux] Error 2
> make: *** [bzImage] Error 2
[...]

I just saw http://www.spinics.net/lists/kernel/msg1869328.html which
fixes things for me.  Sorry for the noise.

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

* Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd
  2014-11-18  2:23   ` Greg Thelen
@ 2014-11-19 16:26     ` H. Peter Anvin
  0 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2014-11-19 16:26 UTC (permalink / raw)
  To: Greg Thelen, Junjie Mao
  Cc: Kees Cook, linux-kernel, Thomas Gleixner, Ingo Molnar, x86,
	Josh Triplett, Matt Fleming, Ard Biesheuvel, Vivek Goyal,
	Andi Kleen, Fengguang Wu, stable

On 11/17/2014 06:23 PM, Greg Thelen wrote:
> 
> On Mon, Nov 17 2014, Greg Thelen wrote:
> [...]
>> Given that bss and brk are nobits (i.e. only ALLOC) sections, does
>> file_offset make sense as a load address.  This fails with gold:
>>

It really doesn't.  We have that information elsewhere.

	-hpa



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

end of thread, other threads:[~2014-11-19 16:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-31 13:40 [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd Junjie Mao
2014-10-31 16:11 ` Kees Cook
2014-11-01 21:24 ` [tip:x86/urgent] " tip-bot for Junjie Mao
2014-11-18  2:17 ` [PATCH v3] " Greg Thelen
2014-11-18  2:23   ` Greg Thelen
2014-11-19 16:26     ` H. Peter Anvin

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