Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-19 Thread H. Peter Anvin 
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


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-19 Thread H. Peter Anvin 
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


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-17 Thread Greg Thelen 

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-17 Thread Greg Thelen 
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
>
> 0x0fe0  --++  <-- 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 
> Signed-off-by: Junjie Mao 
> [kees: switched to perl to avoid hex translation pain in mawk vs gawk]
> [kees: calculated overlap without relocs table]
> Signed-off-by: Kees Cook 
> Cc: sta...@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 ..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  8100  0100  0020  2**12
  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss  000e  81fe8000  01fe8000  013e8000  2**12
  ALLOC
 27 .brk  00026000  820c8000  020c8000  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  8100  0100  1000  2**12
  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss  000e  81feb000  01feb000  00e9  2**12
  ALLOC
 27 .brk  00026000  820cb000  020cb000  00f7  2**0
  ALLOC
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-17 Thread Greg Thelen 
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

 0x0fe0  --++  -- 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...@intel.com
 Signed-off-by: Junjie Mao eternal@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 keesc...@chromium.org
 Cc: sta...@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 ..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  8100  0100  0020  2**12
  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss  000e  81fe8000  01fe8000  013e8000  2**12
  ALLOC
 27 .brk  00026000  820c8000  020c8000  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  8100  0100  1000  2**12
  CONTENTS, ALLOC, LOAD, READONLY, CODE
[...]
 26 .bss  000e  81feb000  01feb000  00e9  2**12
  ALLOC
 27 .brk  00026000  820cb000  020cb000  00f7  2**0
  ALLOC
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-11-17 Thread Greg Thelen 

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.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-10-31 Thread Kees Cook 
On Fri, Oct 31, 2014 at 6:40 AM, 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
>
> 0x0fe0  --++  <-- 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 
> Signed-off-by: Junjie Mao 
> [kees: switched to perl to avoid hex translation pain in mawk vs gawk]
> [kees: calculated overlap without relocs table]
> Signed-off-by: Kees Cook 
> Cc: sta...@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 */
> lealz_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 */
> calldecompress_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

Re: [PATCH v3] x86, kaslr: Prevent .bss from overlaping initrd

2014-10-31 Thread Kees Cook 
On Fri, Oct 31, 2014 at 6:40 AM, Junjie Mao eternal@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

 0x0fe0  --++  -- 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...@intel.com
 Signed-off-by: Junjie Mao eternal@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 keesc...@chromium.org
 Cc: sta...@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 */
 lealz_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 */
 calldecompress_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 */